Trusted Types compatibility: Make VSCode TrustedTypes compliant

[annkamsk]

Hi, I’m Anna, working as an intern on the Google Security team this summer. I’m working on a project that’s part of the larger effort to harden popular open-source libraries against web vulnerabilities.

Following up on an email exchange with @egamma and @jrieken, we’re planning to add Trusted Types to VSCode’s codebase.

What are Trusted Types

DOM-based cross-site scripting (DOM XSS) is one of the most common web security vulnerabilities, and it's very easy to introduce it in your application.

Trusted Types give you the tools to write, security review, and maintain applications free of DOM XSS vulnerabilities by making the dangerous web API functions secure by default. Trusted Types are supported in Chrome since version 83. For more information see https://web.dev/trusted-types/

Why make VSCode compliant with Trusted Types?

Having dealt with large JS/TS codebases, we (the Google ISE team) noticed that while correctly escaping the data is relatively easy - what is hard is assuring that this is done comprehensively. For example, that all "scripty" sinks in innerHTML are covered, and that the escaping is done every time the data reaches them. That's where Trusted Types help tremendously. In our estimations, Trusted Types might effectively kill XSS for serverless applications.

VSCode is a large codebase running in a very sensitive application, having the assurance that it is free of XSS seems like a worthy goal.

Approximate cost of the effort

After running a static analysis pass for Trusted Types compliance on VSCode, I discovered 103 potential issues, ranging from trivial refactorings (assigning non-html literal strings to innerHTML, ~45% of issues) to much more complex cases (building arbitrary html).
I’m planning to start fixing the simplest ones that can be changed without creating a Trusted Types Policy and then take on less trivial problems. Each change will group issues with similar causes and be tracked by a separate more specific github issue.

You can perform the check for Trusted Types compliance on VSCode yourself by running this fork of the VSCode, where I added a script that does exactly that.

Edit: Tagging @engelsdamien and @koto for visibility

[engelsdamien]

Hi there, @koto and myself are mentoring Anna for this project.

Please let us know if there is any process you'd like us to follow for PRs (in addition to the normal contribution guidelines)

[jrieken]

@annkamsk Thanks so far. I went through the usages of innerHTML and to make it simpler for us to assign and review PRs I suggest the following plan

1. Lots of innerHTML-use is coming from renderCodicons. We should have an equivalent that returns elements instead of a string and adopt it. Given this will be a uniform adoption it's OK to put this into a single PR.
2. Another common innerHTML-use is updating style elements, like this. This is part of our theming story and I believe usages can be replaced with textContent. This should also be a relatively uniform change and OK to have in a single PR.
3. Other innerHTML-use. Since these changes will require more review it's best to create more fine grained PRs so that owners can do the review. A good rule of thumb would a PR per feature folder, like 'breadcrumbs' or 'codelens'.

Unfortunately, the last item means a little more overhead. An alternative (and that's how we have often done such changes in the past) is to create and manage a list of pending adoptions and to assign people to fix them. That guarantees a broad understanding of the issue since the whole team will be involved. Generally, the team is happy to make those changes but it needs some tool to discover invalid innerHTML-usages (I have been using TypeScript reference search).

#HappyCoding