Skip to content
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

Add support for / use TrustedTypes (CSP) #850

Open
tomastrajan opened this issue Jun 5, 2024 · 5 comments
Open

Add support for / use TrustedTypes (CSP) #850

tomastrajan opened this issue Jun 5, 2024 · 5 comments
Labels
next major: breaking change this is a change that we should wait to bundle into the next major version type: feature request ‘Nice-to-have’ improvement, new feature or different behavior or design.

Comments

@tomastrajan
Copy link

tomastrajan commented Jun 5, 2024

Describe the solution you'd like
The @googlemaps/js-api-loader should support CSP TrustedTypes

Describe alternatives you've considered
Disabling CSP

Additional context
When CSP is enabled on the server, there will be a runtime error

TypeError: Failed to set the 'src' property on 'HTMLScriptElement': This document requires 'TrustedScriptURL' assignment.

In the library index.mjs file, line #369

a.src = this.url + ? + e; this assignment is the main problem

Tt should have been something like script.src = getPolicy()?.createScriptURL(url) ?? url; instead
and the library should register its TrustedType policy out of the box

@tomastrajan tomastrajan added triage me I really want to be triaged. type: feature request ‘Nice-to-have’ improvement, new feature or different behavior or design. labels Jun 5, 2024
@tomastrajan tomastrajan changed the title Add support for / use TrustedTypes CSP Add support for / use TrustedTypes (CSP) Jun 5, 2024
@usefulthink
Copy link
Contributor

Thanks for the sugestion, I haven't heard of those yet. We will consider adding support for this in the next major version.

Until then, it appears you can - instead of turing CSP / TrustedTypes off entirely - define a default policy that could for example only allow whitelisted URLs to be used as script-URL: https://w3c.github.io/trusted-types/dist/spec/#default-policy-hdr

What I'm not understanding is this: If we can just create a policy to allow our script to load, what is the benefit here? Surely creating new policies has to be limited somehow?

@tomastrajan
Copy link
Author

hey hey @usefulthink

thank you very much for coming back to me!

Yes, we currently work around this by creating a default policy.

What I'm not understanding is this: If we can just create a policy to allow our script to load, what is the benefit here? Surely creating new policies has to be limited somehow?

In general it's all about being able to enable policy for trusted libraries on case by case basis, so if we know that the googlemaps/js-api-loader only assigns src to a script dynamically in case it's accessing its own trusted endpoint, we can enable just that policy, without allowing something potentially malicious (as default would allow for as any other lib could hijack default)

I would propose something like googlemaps-js-api-loader#script or similar.

For more info, you can check Angular docs which deals with this as well as for examples of what a good policy name could look like.

Thank you!

@usefulthink
Copy link
Contributor

Thanks for the information, makes a lot of sense.

Thought about it a bit more and noticed it wont help if just the loader supports a trustedTypes policy: the maps library itself will cause several further violations (scripts being loaded, might also do some string to html operations). So as long as the main maps API hasn't implemented support for using a trustedTypes policy, adding it to the loader won't help much.

I saw you already created an issue in the issuetracker, I'll add a comment there as well https://issuetracker.google.com/issues/342961825

@tomastrajan
Copy link
Author

hey hey @usefulthink !

there now seems to be google-maps-api-loader policy available which means that fix here could make sense ?
image

As far as I understand this is a JS script loaded dynamically by this library, right?

If this library added it's own CSP policy, then we would all be able to remove current workaround (default policy) which solves the...

image

which was reported initially when opening this issue

@usefulthink usefulthink removed the triage me I really want to be triaged. label Oct 1, 2024
@usefulthink
Copy link
Contributor

Thanks for letting me know! I'll look into that – it will likely be added to the upcoming major release.

@usefulthink usefulthink added the next major: breaking change this is a change that we should wait to bundle into the next major version label Oct 24, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
next major: breaking change this is a change that we should wait to bundle into the next major version type: feature request ‘Nice-to-have’ improvement, new feature or different behavior or design.
Projects
None yet
Development

No branches or pull requests

2 participants