Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Mask out username from events #81

Open
bwateratmsft opened this issue Nov 17, 2021 · 4 comments
Open

Mask out username from events #81

bwateratmsft opened this issue Nov 17, 2021 · 4 comments

Comments

@bwateratmsft
Copy link

The username (i.e. literally the OS username) could be considered PII, and should be masked out.

In addition to #80, this would allow us to remove our custom masking logic from https://github.com/microsoft/vscode-azuretools/blob/main/ui/src/masking.ts.

We currently have issues in the above file (microsoft/vscode-azuretools#1024), so it's not quite right to filter out the username unconditionally--maybe, /\bMyUsername\b/ig?

@lramos15
Copy link
Member

This should be in theory be being done by https://github.com/microsoft/vscode-extension-telemetry/blob/main/src/common/baseTelemetryReporter.ts#L180-L261 can you give an example of a property value that is avoiding this?

@bwateratmsft
Copy link
Author

@nturinski / @alexweininger / @wwlorey Do you have an example of usernames leaking through the above linked code? In Docker's case, that code would have caught the items that got through (we didn't have that a high enough version of vscode-extension-telemetry), so I don't have a concrete example.

@lramos15 It's a contrived example but I could see an error message that says something like "User bwateratmsft does not have permission to access foo", which isn't a file path, email, or secret so I think it gets through the above code, unless I'm missing something.

The function of the code here and here is to mask that "bwateratmsft" (however it's being to aggressive).

@lramos15
Copy link
Member

This is a tough one because that user string could be anywhere and as you saw with smaller user strings it can even be in common strings within the data.

@bwateratmsft
Copy link
Author

bwateratmsft commented Nov 29, 2021

I agree. In microsoft/vscode-azuretools#1025 our plan was basically to not mask usernames either 3 or 4 (haven't decided which yet) or less characters. It's definitely possible to still mask something we don't need to, even with that in place, but it should become exponentially less common.

Otherwise, /\bMyUsername\b/ig should probably work as well. Or, a combination of the two.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants