fix(security): add rel="noopener noreferrer" to links opening a new tab - #723
Open
Bhuvanesh66 wants to merge 1 commit into
Open
fix(security): add rel="noopener noreferrer" to links opening a new tab#723Bhuvanesh66 wants to merge 1 commit into
Bhuvanesh66 wants to merge 1 commit into
Conversation
Links rendered with target="_blank" did not set a rel attribute, so the page that opens keeps a working window.opener handle back to podman.io and can navigate the original tab elsewhere. That is reverse tabnabbing, and it also leaks the referrer. The shared Link utility was the root of it. It accepts a target prop but never rendered rel, so every caller passing target="_blank" was affected, including the 404 page. It now defaults rel to "noopener noreferrer" when target is "_blank", while still letting a caller pass an explicit rel. The remaining raw anchors are fixed directly: the seven sponsor logos in ThankYouSection, both article links in ArticleCard (these open URLs returned by the blog.podman.io API), the featured link in Testimonial, and one asciinema link in an older blog post. Verified against the generated HTML rather than only the source: after yarn build, no anchor with target="_blank" is left without rel on any page in build/. Fixes: containers#671 Fixes: containers#576 Signed-off-by: Bhuvanesh66 <bhuvaneshms60@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Concisely describe the change
External links that open in a new tab were missing a
relattribute. When a link usestarget="_blank"with norel, the page that opens gets a workingwindow.openerhandle back to podman.io and can navigate the original tab somewhere else. That is reverse tabnabbing, and it also leaks the referrer.On
mainthere are 12target="_blank"links undersrc/, and only one of them (BlogArticlesList) setsrel.The root of it is the shared
Linkutility insrc/components/utilities/Link/index.tsx. It accepts atargetprop but never renderedrel, so any caller passingtarget="_blank"was affected. The 404 page does exactly that. I changedLinkto defaultreltonoopener noreferrerwhentargetis_blank, and a caller can still pass its ownrelif it needs something different. Fixing it there covers the current caller and any future one, rather than leaving the next person to remember.The rest are plain
<a>tags and are fixed directly:ThankYouSection- the seven sponsor logo linksArticleCard- both article links. These open URLs that come back from theblog.podman.ioAPI, so I would least want those left open.Testimonial- the featured linkblog/2018-10-10-checkpoint-restore.md- one asciinema linkBlogArticlesListalready setrelcorrectly and is not touched. No styling, markup structure, or behaviour changes beyond the added attribute.Fixes: #671
Fixes: #576
Before screenshot / screen recording
Not a visual change, so there is nothing useful to screenshot. On
main:After screenshot / screen recording
With this branch, the same count is 11 literal occurrences plus the 404 page link, which now gets
relfrom theLinkcomponent at render time.I checked the generated HTML rather than just the source. After
yarn build, across every page inbuild/:For example the 404 page link, which is the one that goes through
Link:and a sponsor logo:
yarn buildcompletes successfully andbuild/index.htmlis produced, which is what the PR workflow checks.Checklist
Ensure you have completed the following checklist for your pull request to be reviewed:
commits. (
git commit -s). (If needed, usegit commit -s --amend). The author email must matchthe sign-off email address. See CONTRIBUTING.md
for more information.
Fixes: #00000in commit message (if applicable)@TomSweeneyRedHat @ashley-cui when you get a chance, could one of you take a look? Happy to split the
Linkchange out into its own commit if you would rather review it separately.