feat: adding support for data uri sanitisation

[sahil-seth]

Changes

This PR aims to address this feature request #31719 where we can now sanitise possible data uris coming in the logs.

Context

• Support sanitising data uris #31719

Documentation (please check one with an [x])

• No documentation update is required

How I've tested my work (please select one)

I have verified these changes via:

• Newly added/modified unit tests, or

[sahil-seth]

@rarkins should we go ahead with this change?
The workflows seem to be waiting an approval from a maintainer

[rarkins]

These Regex seem too aggressive and will match some unintentional/irrelevant data messages too

[sahil-seth]

@rarkins Good point, but do you have any examples that you can think of, which might match to the new regex but should not ideally?
Asking cause it might help me optimise the regex quicker.

[rarkins]

No, because there are infinite examples possible, so you should provide some examples of where data uri's need redacting and we make the Regex tighter. For example should it occur only at the start of a field?

[sahil-seth]

Here are some data URI examples which I aimed to redact with this change:

data:application/pkcs8;kid=example-key-123;base64,MIIBIjANBgkqhkiG9w0BAp1s1F3lQfZ9c/GbE=
data:application/jwt;kid=jwt-key-789;base64,eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9
data:application/x-pem-file;kid=pem-cert-202;base64,LS0tLS1CZWdpbiBDRVJUSUZJQ0FURS0tLS0t
data:audio/wav;kid=audio-key-707;base64,UklGRjIAAABXQVZFZm10IBAAAAABAAEAESsAACJWAAACABAAZGF0YYAAAA

Also, upon a quick check with an AI assistant, the regex doesn't seem to match unnecessary strings apart from URLs or data URI like fields.

[sahil-seth]

For example should it occur only at the start of a field?

Yes, it can. It might be safer to redact the whole string after data cause it takes away the knowledge of what kind of key it is, whether it is jwt or pkcs8 or pem-file along with the key itself.

WDYT?

[rarkins]

Please provide a full log example from today (you can redact parts of it yourself). Eg where does this appear today?

[sahil-seth]

Here is an example

[rarkins]

Great, so can we start with the case where we match against ^data: and it's the whole value of the log message? (as in your examples)

[sahil-seth]

@rarkins I'm not sure If I got you this time, could you please elaborate on the suggestion?

[sahil-seth]

@rarkins just following up on above message,
do you mean we update the regex to include the strings starting with data: ?

[rarkins]

Yes, the regex should be strict to match only where the log content starts and ends with a data URI

[viceice]

we can even more tighten the regex to also require the content-type after data:

^data:[0-9a-z-]+/[0-9a-z-]+;.+

[sahil-seth]

@rarkins @viceice
Just to clarify and make sure we are on the same page,

Are we talking about dataUriCredRe OR urlRe when we talk about tightening the regex?

[rarkins]

It's better if you revert your changes to urlRe

[sahil-seth]

@rarkins I've reverted the change to urlRe and added a "if" case for data URIs using the regex suggested above, LMK if this looks fine?

[viceice]

please run prettier to fix linter

[sahil-seth]

@viceice done!

[sahil-seth]

@rarkins @viceice Does this PR looks good to merge now?
once the pipelines are triggered...

automerge enabled

[sahil-seth]

@rarkins @viceice thanks for enabling auto-merge but looks like the pipelines need to be run once again?

[viceice]

@rarkins @viceice thanks for enabling auto-merge but looks like the pipelines need to be run once again?

stop merging main, we're using GitHub merge queue. if you push something then actions need new approval because you're a first time contributer

[renovate-release]

🎉 This PR is included in version 38.111.0 🎉

The release is available on:

• GitHub release
• npm package (@latest dist-tag)
• 38.111.0

Your semantic-release bot 📦🚀