Azure / azure-sdk-for-js

This repository is for active development of the Azure SDK for JavaScript (NodeJS & Browser). For consumers of the SDK we recommend visiting our public developer docs at https://docs.microsoft.com/javascript/azure/ or our versioned developer docs at https://azure.github.io/azure-sdk-for-js.
MIT License
2.06k stars 1.19k forks source link

[test] vitest can leak secrets in pipeline output #29630

Open timovv opened 4 months ago

timovv commented 4 months ago

When a test fails with an error, vitest outputs a JSON serialization of the error object to console. This is helpful when debugging, but can cause information that would have otherwise been sanitized to be output in pipeline runs, like in this example where an access token showed up (MS internal link): https://dev.azure.com/azure-sdk/internal/_build/results?buildId=3768430&view=logs&j=8f098e13-557e-5a76-9331-06424800e0fa&t=5005ede9-2880-5bbf-ce42-ae979164a4d1&l=105. Is it possible to suppress this output or sanitize it somehow in the pipeline?

Cc @mpodwysocki

xirzec commented 4 months ago

/cc @jeremymeng

jeremymeng commented 4 months ago

I wonder whether we should remove the request object from the RestError object, or sanitize it by default?

timovv commented 4 months ago

I feel like there are definitely production use cases where customers would want to see the content of the request in an error case. Sometimes there may be secrets in the response as well, which is an even more useful property. Maybe it would be possible to sub in a different mock implementation of RestError during tests that doesn't have this information, or sanitizes things? (this is if Matt's current PR doesn't fix the problem, I haven't had a chance to test that yet)

jeremymeng commented 4 months ago

I am a little concerned because error objects most likely get logged all the time.

timovv commented 4 months ago

Yeah I agree that is a concern. I guess there are a few different ways people could log the error: