Skip to content

feat(core): add API to provide CSP nonce for inline stylesheets#49444

Closed
crisbeto wants to merge 1 commit intoangular:mainfrom
crisbeto:6361/csp-nonce
Closed

feat(core): add API to provide CSP nonce for inline stylesheets#49444
crisbeto wants to merge 1 commit intoangular:mainfrom
crisbeto:6361/csp-nonce

Conversation

@crisbeto
Copy link
Copy Markdown
Member

@crisbeto crisbeto commented Mar 16, 2023

Angular uses inline styles to insert the styles associated with a component. This violates the strict styles Content Security Policy which doesn't allow inline styles by default. One way to allow the styles to be applied is to set a nonce attribute on them, but because the code for inserting the stylesheets is deep inside the framework, users weren't able to provide it without accessing private APIs.

These changes add a new CSP_NONCE injection token that will allow users to provide a nonce, if their app is using CSP. If the token isn't provided, the framework will look for an ngCspNonce attribute on the app's root node instead. The latter approach is provided as a convenience for apps that render the index.html through a server, e.g. <app ngCspNonce="{% randomNonceAddedByTheServer %}"></app>.

This PR addresses adding the nonce to framework-generated styles. There will be follow-up PRs that add support for it in critical CSS tags in the CLI, and in Angular Material.

Fixes #6361.

@angular-robot angular-robot bot added the detected: feature PR contains a feature commit label Mar 16, 2023
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unrelated change, I just saw that we could get the head directly without querying the DOM. I left the old approach as a fallback in case it was there as a workaround for Domino.

@crisbeto crisbeto added action: review The PR is still awaiting reviews from at least one requested reviewer area: core Issues related to the framework runtime target: major This PR is targeted for the next major release labels Mar 16, 2023
@ngbot ngbot bot modified the milestone: Backlog Mar 16, 2023
@crisbeto crisbeto modified the milestones: Backlog, v16-candidates Mar 16, 2023
@crisbeto crisbeto force-pushed the 6361/csp-nonce branch 3 times, most recently from 5e3b71e to 92db19c Compare March 16, 2023 14:43
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This uses @Inject instead of inject, because we had a bunch of tests that were constructing classes manually which lacked a DI context.

@crisbeto crisbeto marked this pull request as ready for review March 16, 2023 15:03
@pullapprove pullapprove bot requested a review from jessicajaniuk March 16, 2023 15:04
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

JFYI, the ServerStyleHost is being removed in #49424, so we'll need to update the location of this logic.

Copy link
Copy Markdown
Contributor

@jessicajaniuk jessicajaniuk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Aside from @AndrewKushnir's comment, this looks good to me

reviewed-for: fw-core, fw-platform-server, public-api

@jessicajaniuk jessicajaniuk removed their request for review March 16, 2023 16:49
@pullapprove pullapprove bot requested a review from dylhunn March 16, 2023 16:49
Copy link
Copy Markdown
Contributor

@dylhunn dylhunn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

reviewed-for: public-api

Copy link
Copy Markdown
Member

@alxhub alxhub left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed-for: public-api

@crisbeto crisbeto removed the request for review from pkozlowski-opensource March 17, 2023 05:51
@crisbeto crisbeto added action: merge The PR is ready for merge by the caretaker and removed action: review The PR is still awaiting reviews from at least one requested reviewer labels Mar 17, 2023
crisbeto added a commit to crisbeto/angular-cli that referenced this pull request Mar 17, 2023
Companion change to angular/angular#49444. Adds an HTML processor that finds the `ngCspNonce` attribute and copies its value to any inline `style` tags in the HTML. The processor runs late in the processing pipeline in order to pick up any `style` tag that might've been added by other processors (e.g. critical CSS).
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OOC, does closure minify this property?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure, but I wanted to keep it safe since I've seen it minify things like el.style.foo before.

Angular uses inline styles to insert the styles associated with a component. This violates the strict styles [Content Security Policy](https://web.dev/strict-csp/) which doesn't allow inline styles by default. One way to allow the styles to be applied is to set a `nonce` attribute on them, but because the code for inserting the stylesheets is deep inside the framework, users weren't able to provide it without accessing private APIs.

These changes add a new `CSP_NONCE` injection token that will allow users to provide a nonce, if their app is using CSP. If the token isn't provided, the framework will look for an `ngCspNonce` attribute on the app's root node instead. The latter approach is provided as a convenience for apps that render the `index.html` through a server, e.g. `<app ngCspNonce="{% randomNonceAddedByTheServer %}"></app>`.

This PR addresses adding the nonce to framework-generated styles. There will be follow-up PRs that add support for it in critical CSS tags in the CLI, and in Angular Material.

Fixes angular#6361.
@pkozlowski-opensource
Copy link
Copy Markdown
Member

This PR was merged into the repository by commit 17e9862.

crisbeto added a commit to crisbeto/angular-cli that referenced this pull request Mar 20, 2023
Companion change to angular/angular#49444. Adds an HTML processor that finds the `ngCspNonce` attribute and copies its value to any inline `style` tags in the HTML. The processor runs late in the processing pipeline in order to pick up any `style` tag that might've been added by other processors (e.g. critical CSS).
angular-robot bot pushed a commit to angular/angular-cli that referenced this pull request Mar 20, 2023
Companion change to angular/angular#49444. Adds an HTML processor that finds the `ngCspNonce` attribute and copies its value to any inline `style` tags in the HTML. The processor runs late in the processing pipeline in order to pick up any `style` tag that might've been added by other processors (e.g. critical CSS).
alan-agius4 added a commit to alan-agius4/universal that referenced this pull request Mar 30, 2023
Companion change to angular/angular#49444. That adds support to add nonce attributes on inline styles
alan-agius4 added a commit to alan-agius4/universal that referenced this pull request Mar 30, 2023
Companion change to angular/angular#49444. That adds support to add nonce attributes on inline styles
alan-agius4 added a commit to alan-agius4/universal that referenced this pull request Mar 30, 2023
Companion change to angular/angular#49444. That adds support to add nonce attributes on inline styles
alan-agius4 added a commit to alan-agius4/universal that referenced this pull request Mar 30, 2023
Companion change to angular/angular#49444. That adds support to add nonce attributes on inline styles
alan-agius4 added a commit to alan-agius4/universal that referenced this pull request Mar 31, 2023
Companion change to angular/angular#49444. That adds support to add nonce attributes on inline styles
alan-agius4 added a commit to alan-agius4/universal that referenced this pull request Mar 31, 2023
Companion change to angular/angular#49444. That adds support to add nonce attributes on inline styles
alan-agius4 added a commit to alan-agius4/universal that referenced this pull request Mar 31, 2023
Companion change to angular/angular#49444. That adds support to add nonce attributes on inline styles
alan-agius4 added a commit to alan-agius4/universal that referenced this pull request Mar 31, 2023
Companion change to angular/angular#49444. That adds support to add nonce attributes on inline styles
alan-agius4 added a commit to angular/universal that referenced this pull request Mar 31, 2023
Companion change to angular/angular#49444. That adds support to add nonce attributes on inline styles
@angular-automatic-lock-bot
Copy link
Copy Markdown

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 Apr 17, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

action: merge The PR is ready for merge by the caretaker area: core Issues related to the framework runtime detected: feature PR contains a feature commit target: major This PR is targeted for the next major release

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Inline <style> elements violates style-src Content Security Policy

7 participants