Support Trusted Types from DOM lib

[tosmolka]

We are still waiting on DOM lib to fully support Trusted Types (microsoft/TypeScript#30024). I was proposing a fix in microsoft/TypeScript-DOM-lib-generator#1246 but it might take a while to get this merged in and released (= Safari or Firefox start supporting TTs).

I ran tsec against codebase that was using this updated DOM lib and found out that tsec does not recognize these built-in TT types.

For one, the matcher expects the types to be defined in DefinitelyTyped (@types/trusted-types):

tsec/src/third_party/tsetse/util/trusted_types_configuration.ts

Line 27 in a04e15a

modulePathMatcher: '/node_modules/@types/trusted-types/',

It would be great if we could add this support to tsec for people (like us) who are already using DOM lib with TTs in their code.

[uraj]

Thanks for reporting this. Due to internal issues, tsec can't accept external contributions (as stated in our CONTRIBUTING.md). I will patch this internally and release a new version.

[uraj]

Could you provide some details about your setup? Did you swap out the original lib.dom.d.ts and use an updated one? Or is the updated lib.dom.d.ts released as an npm package?

[tosmolka]

@uraj , I ended up patching lib.dom.d.ts in my local setup, I pushed patch to 27256dc as a reference.

[tosmolka]

@uraj , do you have enough info to investigate this further? Any updates? Thanks.

[uraj]

@tosmolka Sorry we're currently working on adding ESLint support. It should be done by the end of this week. After that I will add support for TT in DOM lib.

Another thing that might be useful to know: in which file do you define the Trusted Types? Did you put the definition in lib.dom.d.ts or you have a separate file for Type declaration?

[uraj]

@tosmolka I'm about to release a patch. Before that, want to confirm with you on how you amended lib.dom.d.ts. Did you just append something like below to the file?

declare class TrustedHTML {
  //...
}

[tosmolka]

@uraj , I used this: 27256dc and applied patch using patch-package.

[uraj]

Should be fixed by 1ceed96. Will publish a new version soon.

[uraj]

Please try 0.2.4.

[tosmolka]

@uraj , I did a quick test and I think this needs little bit of more work. I tried with HTMLScriptElement.src sink and tsec still flags when I do assignments with string | TrustedScriptURL or TrustedScriptURL types.

I think I faced similar issues when I proposed the PR and I fixed that by adding isTrustedType and isTrustedTypeUnionWithString methods to is_trusted_type.ts.

[uraj]

@tosmolka OK I got what you meant. I will discuss with the team and see whether we should support this form of assignments. With 0.2.4 you can now at least write

declare const trustedScriptUrl: string | TrustedScriptURL;
script.src = trustedScriptUrl as string;

where TrustedScriptURL is defined in the patched lib.dom.d.ts.

[tosmolka]

@uraj , do you have any updates regarding support for this form of assignments? Thanks a lot.

declare const trustedScriptUrl: string | TrustedScriptURL;
script.src = trustedScriptUrl;

[uraj]

@tosmolka With lib.dom.d.ts patched, there is no longer the need to use string | TrustedScriptURL. I think we can support the following

declare const trustedScriptUrl: TrustedScriptURL;
script.src = trustedScriptUrl;

Is that OK to you?

[tosmolka]

I think we will continue using string | TrustedScriptURL in certain cases (e.g. Trusted Types are not enabled/supported).

Now, if tsec keeps flagging this as an issue because string MIGHT flow into this sink - that's understandable and we would have to deal with it on our side.

But then I'd like tsec to be consistent and start flagging also this case:

declare const trustedScriptUrl: string | TrustedScriptURL;
script.src = trustedScriptUrl as string;

WDYT?

[uraj]

We can flag that case, if we detect that TrustedScriptURL is defined as an ambient type, i.e., an indicator that lib.dom.d.ts is patched. We should really check if script.src actually accepts an TrustedScriptURL but that seems a much more complicated change.

[uraj]

@tosmolka I've submitted the patch, but for some reason I can't publish a new version right now. I will try over the weekends, but you can try out the patch before that if you want

[uraj]

0.2.6 is now published. Let me know if the new version doesn't work for your setup.

[tosmolka]

@uraj , I quickly checked the change.

It seems to be working fine for intersection types - string & TrustedScriptURL . But what we need are union types - string | TrustedScriptURL - this is still being flagged by tsec as an issue.

[uraj]

@tosmolka this is intended. I thought your project is OK with this, per our previous discussions.

[tosmolka]

@uraj , I thought we were discussion union types - string | TrustedScriptURL - this is what we are using in our projects, this is what I . I am confused now, are you saying we should change our setup or are you saying you are going to fix this in tsec? Can you add unit tests for these cases into tsec so that the expected behavior is clearer? Thanks.

[uraj]

@tosmolka We were indeed talking about union types. The new behavior is that, if tsec found the definition of Trusted Types is ambient, union types like string|TrustedScriptURL are not longer acceptable at the sinks, since that may cause problems. For consistency, the following case will be flagged as well:

declare const trustedScriptUrl: string | TrustedScriptURL;
script.src = trustedScriptUrl as string;

On unit tests: we do have them internally, but for some reason we don't export the test code to github.

[tosmolka]

@uraj , OK, I misunderstood what is the intended behavior after your fix. Would be good to update docs accordingly at some point. Thanks.