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

Latest version requires unsafe-inline due to inline styles #4445

Open
rupertbg opened this issue Mar 23, 2023 · 12 comments · Fixed by #4611
Open

Latest version requires unsafe-inline due to inline styles #4445

rupertbg opened this issue Mar 23, 2023 · 12 comments · Fixed by #4611
Labels
help wanted type/enhancement Features or improvements to existing features

Comments

@rupertbg
Copy link

Content Security Policies need to be set to 'unsafe-inline' to work with xterm.js. Older versions didn't use inline styles so this wasn't an issue.

Ideally xterm should stop using inline styles or support a user-provided nonce value that can be set in the CSP. See https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Content-Security-Policy/style-src

Details

  • Browser and browser version: all
  • OS version: all
  • xterm.js version: 5.1.0

Steps to reproduce

  1. Set a content security policy like "style-src 'self';"
  2. Make an xterm that has a resizable container
  3. Resizing causes CSP errors in the console.
  4. Resizing doesn't work properly
@Tyriar
Copy link
Member

Tyriar commented Mar 23, 2023

It always needed it depending on the renderer you are using. Is the proposal that the nonce is generated by the server and passed into xterm.js as an option?

@Tyriar Tyriar added the type/enhancement Features or improvements to existing features label Mar 23, 2023
@Codehardt
Copy link

Same issue here, properties like font-family are blocked by CSP in newest version 5.2.1. I would be fine if I can pass a server-generated nonce to the Terminal constructor.

Kind regards,
Marcel

@jerch
Copy link
Member

jerch commented Jul 20, 2023

What would be the best way forward here? As I have lit. no clue about CSP scoping rules, some help from more experienced CSP-focused devs would be appreciated.

  • How does CSP interact with JS inline .style access? Should we rework all those to explicit style elements with a nounce passed through? (That would be quite a big refactoring...)
  • Ideally we find the most restrictive still working set of CSP directives, and rework the code accordingly (e.g. passing nounces into xtermjs).
  • In the end we prolly should create some docs about CSP usage with xtermjs and the catches.

@rupertbg
Copy link
Author

Well one option would be to use stylesheets instead of inline styles, which is the reason it became a problem in the first place.

@jerch
Copy link
Member

jerch commented Jul 20, 2023

Well one option would be to use stylesheets instead of inline styles, which is the reason it became a problem in the first place.

Thats not possible for all styling parts, as some are calculated from metrics in JS.

@rupertbg
Copy link
Author

Well one option would be to use stylesheets instead of inline styles, which is the reason it became a problem in the first place.

Thats not possible for all styling parts, as some are calculated from metrics in JS.

You could consider updating css variables via setProperty on the :root instead of applying inline styles. I believe this does not count for the purposes of CSP.

@Codehardt
Copy link

The only thing that CSP is blocking if you use inline <style>...</style> or inline style="...". But there are still ways to dynamically set styles with JS that are not blocked by CSP, by using elem.setProperty(<prop>, <value>) or elem.style.<prop> = <value>. If this is no option, then there is also a nonce possible: Just allow passing a nonce string in the Terminal constructor and use this nonce in all <style> sections: <style nonce="<the nonce>">...</style>.

@Tyriar
Copy link
Member

Tyriar commented Jul 20, 2023

But there are still ways to dynamically set styles with JS that are not blocked by CSP, by using elem.setProperty(, ) or elem.style. = .

This is not an option as styling every single character like this would hurt performance.

Just allow passing a nonce string in the Terminal constructor and use this nonce in all <style> sections: <style nonce="">...</style>.

This sounds fine, let's add a nonce/cspNonce option?

@SimonSiefke
Copy link
Contributor

Maybe the CSSStyleSheet api could useful for this, for example:

const sheet = new CSSStyleSheet();
sheet.replaceSync("a { color: red; }");
document.adoptedStyleSheets.push(sheet)

@Tyriar
Copy link
Member

Tyriar commented Jul 24, 2023

@SimonSiefke if that works around the CSP issue we could use that method in the browsers that support it 👍

@Tyriar
Copy link
Member

Tyriar commented Aug 15, 2023

This change was reverted in #4680

@Tyriar Tyriar reopened this Aug 15, 2023
@deadbaed
Copy link

deadbaed commented May 6, 2024

A workaround until CSPs are implemented again: use the webgl addon!

I was getting CSP errors when importing xtermjs in my angular app, and had only a non functional black box. By using the webgl backend I am still getting CSP errors, but now I have a functional xtermjs!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted type/enhancement Features or improvements to existing features
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants