-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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
Strip ansi color codes and do not use bypass HTML function #5737
Conversation
Codecov Report
@@ Coverage Diff @@
## master #5737 +/- ##
==========================================
+ Coverage 44.34% 44.38% +0.04%
==========================================
Files 214 214
Lines 9126 9117 -9
Branches 113 112 -1
==========================================
Hits 4047 4047
+ Misses 4804 4796 -8
+ Partials 275 274 -1
Continue to review full report at Codecov.
|
/lgtm |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: floreks, maciaszczykm The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Hurray! Thanks! Go Team! |
Does this change mean that colours are no longer supported in the logs view? |
@MattJeanes correct. They are stripped from the logs to not impact readability. |
A shame, but understandable given the security concern - thanks for letting me know, not sure if that would be worth a mention in the v2.2.0 release notes? |
npm audit fix
ansi-to-html
and addedstrip-ansi
insteadbypassSecurityTrustHtml
Since every log line was first sanitized before applying ANSI to HTML conversion, I do not think there was a potential XSS vulnerability. The converter itself would have to be compromised in order to append insecure code. In order to prevent that logs will be further stripped from ANSI codes.
Example