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

inlineCritical CSS asynchronous loading method breaks with CSP #20864

Closed
jpduckwo opened this issue May 20, 2021 · 36 comments
Closed

inlineCritical CSS asynchronous loading method breaks with CSP #20864

jpduckwo opened this issue May 20, 2021 · 36 comments

Comments

@jpduckwo
Copy link

Bug Report

Affected Package

@angular/cli

Is this a regression?

Nay

Description

The method used for inlining critical css and asynchronously loading it, breaks and doesn't load the external stylesheet when you have a content security policy that doesn't include script-src 'unsafe-inline'. As the name suggests this isn't a very secure way of operating for various reasons such as script injection.

You can fix by disabling inlineCritical in the optimizations - however maybe there is a better way to load the styles, maybe in a JS file?

<link rel="stylesheet" href="styles.css" media="print" onload="this.media='all'">

It's the onload="this.media='all'" that breaks it

Minimal Reproduction

https://github.com/jpduckwo/ng12-csp-issue

run:
ng serve

Exception or Error

Refused to execute inline event handler because it violates the following Content Security Policy directive: "default-src 'self'". Either the 'unsafe-inline' keyword, a hash ('sha256-...'), or a nonce ('nonce-...') is required to enable inline execution. Note also that 'script-src' was not explicitly set, so 'default-src' is used as a fallback.
image

Your Environment

Angular Version:


@angular-devkit/architect       0.1200.1
@angular-devkit/build-angular   12.0.1
@angular-devkit/core            12.0.1
@angular-devkit/schematics      12.0.1
@schematics/angular             12.0.1
rxjs                            6.6.7
typescript                      4.2.4
@hardikpatel043

This comment has been minimized.

@tiberiuzuld

This comment has been minimized.

@tiberiuzuld
Copy link

Found how to disable the inlineCritical styles
aa3ea88
https://angular.io/guide/workspace-config#styles-optimization-options
In angular.json in the build configuration Instead of

"optimization": true

put

"optimization": {
  "scripts": true,
  "styles": {
    "minify": true,
    "inlineCritical": false
  },
  "fonts": true
},

@alan-agius4 alan-agius4 transferred this issue from angular/angular May 20, 2021
@alan-agius4
Copy link
Collaborator

We can probably solve this by changing the preload strategy of critters from media to default.

Let me reach out to the Chrome team so see if there are any drawbacks of doing so.

@ngbot ngbot bot modified the milestone: needsTriage May 20, 2021
@alan-agius4 alan-agius4 added the feature Issue that requests a new feature label May 20, 2021
@ngbot ngbot bot modified the milestones: needsTriage, Backlog May 20, 2021
@alan-agius4 alan-agius4 added type: bug/fix and removed feature Issue that requests a new feature labels May 20, 2021
@ngbot ngbot bot modified the milestones: Backlog, needsTriage May 20, 2021
@alan-agius4 alan-agius4 added freq1: low Only reported by a handful of users who observe it rarely severity3: broken labels May 20, 2021
@ngbot ngbot bot modified the milestones: needsTriage, Backlog May 20, 2021
@developit
Copy link
Contributor

@alan-agius4 I think the "body" mode could work - it injects styles at the <link> location, then adds the <link rel=stylesheet href=css> just before </body>.

@alan-agius4
Copy link
Collaborator

alan-agius4 commented May 20, 2021

@developit, thanks for the input. I had a chat with @janicklas-ralph, and mentioned that the default is idea although in some cases it can break CSS because of different CSS ordering. I’ll investigate a bit more.

@developit
Copy link
Contributor

@alan-agius4 another option would be to have the JS bundle itself flip those preloads to stylesheets (as a backup). It could scan for the preload/disabled <link> elements, add an onload listener, then force a synchronous load event if the sheet has already been loaded:

[].forEach.call(document.querySelectorAll('link[rel="stylesheet"][media="print"]'), n => {
  n.onload = () => n.media='';
  n.href += ''; // onload if not already loading
});

@alan-agius4 alan-agius4 self-assigned this May 21, 2021
@alan-agius4 alan-agius4 added the needs: discussion On the agenda for team meeting to determine next steps label May 21, 2021
@ghusta
Copy link

ghusta commented May 27, 2021

An interesting article with an overview of this concept :

@AElmoznino
Copy link

Found how to disable the inlineCritical styles
aa3ea88
https://angular.io/guide/workspace-config#styles-optimization-options
In angular.json in the build configuration Instead of

"optimization": true

put

"optimization": {
  "scripts": true,
  "styles": {
    "minify": true,
    "inlineCritical": false
  },
  "fonts": true
},

I tried @tiberiuzuld's solution but didn't get it working. Any other workarounds?

@pacocom
Copy link

pacocom commented Nov 4, 2021

I have similar but not equal problem
Refused to execute inline event handler because it violates the following Content Security Policy directive: "script-src 'self' 'unsafe-eval'". Either the 'unsafe-inline' keyword, a hash ('sha256-...'), or a nonce ('nonce-...') is required to enable inline execution. Note that hashes do not apply to event handlers, style attributes and javascript: navigations unless the 'unsafe-hashes' keyword is present.
image

          "configurations": {
            "production": {
              "budgets": [
                {
                  "type": "anyComponentStyle",
                  "maximumWarning": "6kb"
                }
              ],
              "optimization": {
                "scripts": true,
                "styles": {
                  "minify": true,
                  "inlineCritical": false
                },
                "fonts": true
              },
              "outputHashing": "all",
              "sourceMap": false,
              "namedChunks": false,
              "extractLicenses": true,
              "vendorChunk": false,
              "buildOptimizer": true,
              "serviceWorker": false,
              "fileReplacements": [
                {
                  "replace": "src/environments/environment.ts",
                  "with": "src/environments/environment.prod.ts"
                }
              ]
            }
          },

@jpduckwo
Copy link
Author

jpduckwo commented Nov 4, 2021

Hi @pacocom, it looks like the same problem. Maybe you need to apply the fix to other config areas of you angular config. Look for other "optimization" settings in your angular.json that are set to something other than false.

@pacocom
Copy link

pacocom commented Nov 5, 2021

It works now.
Thanks.

@sander1095
Copy link

@alan-agius4 and maybe others..

What is the status of this issue? What i understand is that we have to disable the inline css feature, OR make our CSP more insecure. Both dont sound like great solutions.

Will there be a good fix to not require us to either sacrifice perf or security? If not, will there be documentation for angular with a CSP guide on how to create a Security performance balance?

@pauldraper
Copy link

pauldraper commented Nov 30, 2021

disable the inline css feature, OR make our CSP more insecure

Just disable inline CSS.

External stylesheets worked for dial-up internet, IDK why people insist on complicating the most simple things when speeds are 100x what they used to be.

EDIT: Enjoy your bugs.

@PapaNappa
Copy link

@sander1095 One work-around is of course to add
script-src 'unsafe-hashes' 'sha256-MhtPZXr7+LpJUY5qtMutB+qWfQtMaPccfe7QXtCcEYc='
This is not really more insecure, as it only allows the this.media='all' script to execute, which is the culprit of this issue.

@silvia-giordano
Copy link

Hello,
I'm on Angular 13.0.2 and I have the same problem as @pacocom. I tried to apply the fix to other config areas of my angular config (as suggested by @jpduckwo ) but still the same error.
Any ideas?

@Rugshtyne
Copy link

Can somebody point to the logic which extracts the 'critical css' and then renders it as inline? I mean maybe it's possible to customize that to include CSP nonces you have defined somewhere.

@rickz21
Copy link

rickz21 commented Apr 16, 2022

I am trying to apply fix in Angular 13. Please post angular.json with fix. I changed it as suggested. But, I still have
body style="overflow: auto; position: static; touch-action: auto;"
I didn't put that inline style there in the body tag. I guess Angular wants a quick page paint.
Please post angular.json with fix. My angular.json file is the following. But, it doesn't stop inlining.
angularjson.txt

@jpduckwo
Copy link
Author

@rickz21 try putting the optimizations in the build > options portion of the JSON

e.g.

...
      "architect": {
        "build": {
          "builder": "@angular-devkit/build-angular:browser",
          "options": {
            "outputPath": "dist",
            "index": "src/index.html",
            "main": "src/main.ts",
            "polyfills": "src/polyfills.ts",
            "tsConfig": "tsconfig.app.json",
            "assets": [
...
              "src/assets"
            ],
            "styles": [
              "src/styles.scss"
            ],
            "scripts": [],
            "outputHashing": "all",
            "optimization": {
              "scripts": true,
              "styles": {
                "minify": true,
                "inlineCritical": false
              },
              "fonts": true
            },
...

@rickz21
Copy link

rickz21 commented Apr 19, 2022

@jpduckwo thank you trying to help me. I did paste in the lines you posted within build.options but it didn't work for me. Angular still inserted a style attribute into my page's body tag.
body style="overflow: auto; position: static; touch-action: auto;"
In the CSP report, Chrome and Edge browsers suggested that I use
style-src 'self' 'sha256-47DEQpj8HBSa+/TImW+5JCeuQeRkm5NMpJWZG3hSuFU=';
and that does work to avoid that violation. Also, I am depending on ngx-image-cropper. That package inserts some inline styling as well. That styling seems to change with each page request. So, it doesn't seem possible to use a hash for them. Hopefully, Angular 14 will work better for me. Thanks again.

@rickz21
Copy link

rickz21 commented Apr 19, 2022

I read
angular/angular#6361
angular/components#24633
and see that there are different opinions.

@jdavidhermoso
Copy link

https://0xdbe.github.io/AngularSecurity-DisableInlineCriticalCSS/

@rickz21
Copy link

rickz21 commented May 5, 2022

@jdavidhermoso I tried that configuration. It didn't stop Angular from adding inline styling to my body tag.

@jpduckwo
Copy link
Author

jpduckwo commented May 5, 2022

Hey @rickz21 the style tag on the body isn't related to this issue. This one is about angular automatically inlining critical css. It sounds like your styles on the body could be coming from something else. Maybe it's another dependency? Might be worth posting a new issue if you can create minimal reproduction

@rickz21
Copy link

rickz21 commented May 6, 2022

@jpduckwo Angular is the only dependency. I will do as you suggest. I will try to create a minimal demonstration.

@ZenwalkerD
Copy link

Hello All,

In my case, my project uses Angular 13 latest; and this issue was happening only on PROD build. Not on Dev/Staging builds. I searched angular documentation; no where it is mentioned why only production grade build gets affected?

Any clarity would enlighten me on this.

Thanks

@jpduckwo
Copy link
Author

@ZenwalkerD check your angular.json, your settings for dev/staging will have a different optimization setting than prod. That's my guess :)

@jprivet-dev
Copy link

Thank's @tiberiuzuld 🎉

I used your solution #20864 (comment) for a Chrome extension created with angular@14.0.2. I also needed the code generated by the ng build command to have no inline script.

I put the following optimization configuration in my angular.json file:

          "configurations": {
            "production": {
              "...": "...",
              "optimization": {
                "scripts": true,
                "styles": {
                  "minify": true,
                  "inlineCritical": false
                },
                "fonts": true
              }
            },
            "development": {
              "...": "..."
            }
          },

lberrymage added a commit to accrescent/devconsole that referenced this issue Dec 7, 2022
The changes to angular.json are to disable inline critical CSS, which
breaks without "script-src 'unsafe-inline'" (which we obviously don't
want to have). See
angular/angular-cli#20864 for details.
@alan-agius4
Copy link
Collaborator

Thanks for reporting this issue. This issue is now obsolete due to changes in the recent releases.

See: #24903

@angular-automatic-lock-bot
Copy link

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators May 2, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.