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.

Sign up for GitHub

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

Jump to bottom

docs: update security guide to mention new CSP functionality #49561

Closed
wants to merge 1 commit into from
Closed

docs: update security guide to mention new CSP functionality #49561

wants to merge 1 commit into from

Conversation

crisbeto
Copy link
Member

Updates the security guide to reflect the recently-added CSP APIs.

@crisbeto crisbeto added area: docs action: review The PR is still awaiting reviews from at least one requested reviewer target: major This PR is targeted for the next major release labels Mar 23, 2023
@crisbeto crisbeto added this to the v16-candidates milestone Mar 23, 2023
@crisbeto crisbeto requested a review from jelbourn March 23, 2023 09:22
@github-actions
Copy link

github-actions bot commented Mar 23, 2023 •
edited

Deployed aio for 4723829 to: https://ng-dev-previews-fw--pr-angular-angular-49561-oe0j25p8.web.app

Note: As new commits are pushed to this pull request, this link is updated after the preview is rebuilt.

@crisbeto
Copy link
Member Author

Updated the guide to reflect the changes from angular/angular-cli#24903.

jelbourn
jelbourn reviewed Mar 29, 2023
View reviewed changes
aio/content/guide/security.md Outdated Show resolved Hide resolved
aio/content/guide/security.md Outdated Show resolved Hide resolved
aio/content/guide/security.md Outdated Show resolved Hide resolved
aio/content/guide/security.md Outdated Show resolved Hide resolved
aio/content/guide/security.md Outdated
Comment on lines 202 to 206
| Sections | Details |
|:--- |:--- |
| `default-src 'self';` | Allows the page to load all its required resources from the same origin. |
| `style-src 'self' 'nonce-randomNonceGoesHere';` | Allows the page to load global styles from the same origin \(`'self'`\) and ones inserted by Angular with the `nonce-randomNonceGoesHere`. |
| `script-src 'self' 'nonce-randomNonceGoesHere';` | Allows the page to load JavaScript from the same origin \(`'self'`\) and ones inserted by the Angular CLI with the `nonce-randomNonceGoesHere`. This is only required if you're using critical CSS inlining. |
Copy link
Member

Choose a reason for hiding this comment

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

I don't think we should include script-src with a nonce here?

Copy link
Member Author

Choose a reason for hiding this comment

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

The script-src is necessary when using critical CSS inlining. It works by setting media="print" on link tags and setting it back to media="all" after it has loaded. See angular/angular-cli#24903.

aio/content/guide/security.md Outdated Show resolved Hide resolved
@pullapprove pullapprove bot requested a review from jelbourn March 29, 2023 17:53
@crisbeto
docs: update security guide to mention new CSP functionality
4723829 
Updates the security guide to reflect the recently-added CSP APIs.
@crisbeto
Copy link
Member Author

Feedback has been addressed.

jelbourn
jelbourn approved these changes Mar 30, 2023
View reviewed changes
Copy link
Member

@jelbourn jelbourn left a comment

Choose a reason for hiding this comment

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

LGTM

@pullapprove pullapprove bot requested a review from jelbourn March 30, 2023 18:31
alxhub
alxhub approved these changes Mar 30, 2023
View reviewed changes
Copy link
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: global-approvers

@crisbeto crisbeto added action: merge The PR is ready for merge by the caretaker merge: caretaker note Alert the caretaker performing the merge to check the PR for an out of normal action needed or note and removed action: review The PR is still awaiting reviews from at least one requested reviewer labels Mar 30, 2023
@crisbeto
Copy link
Member Author

crisbeto commented Mar 30, 2023 •
edited

Caretaker note: the PR has the necessary approvals (see the PullApprove status), but for some reason Github won't allow me to remove the additional review requests.

@angular-robot
Copy link
Contributor

angular-robot bot commented Mar 30, 2023

This PR was merged into the repository by commit f19319e.

@angular-robot angular-robot bot closed this in f19319e Mar 30, 2023
@alan-agius4 alan-agius4 mentioned this pull request Apr 1, 2023
Closed
This was referenced Apr 10, 2023
Closed
Closed
@amanplans amanplans mentioned this pull request Apr 13, 2023
Closed
@A-Fitz-Nelnet A-Fitz-Nelnet mentioned this pull request Apr 19, 2023
Open
@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 Apr 30, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@alan-agius4 alan-agius4 alan-agius4 left review comments

@alxhub alxhub alxhub approved these changes

@josephperrott josephperrott Awaiting requested review from josephperrott

@pkozlowski-opensource pkozlowski-opensource Awaiting requested review from pkozlowski-opensource

@jelbourn jelbourn Awaiting requested review from jelbourn

Assignees
No one assigned
Labels
action: merge The PR is ready for merge by the caretaker aio: preview merge: caretaker note Alert the caretaker performing the merge to check the PR for an out of normal action needed or note target: major This PR is targeted for the next major release
Projects
None yet
Milestone
v16-candidates
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@crisbeto @jelbourn @alxhub @alan-agius4