Add code-preview layout and web component example page#685
Add code-preview layout and web component example page#685juliawu merged 10 commits intodatacommonsorg:masterfrom
Conversation
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly enhances the documentation site by introducing a new display format for web component examples. It allows users to view the source code and a live, interactive preview of a component simultaneously, improving clarity and ease of understanding. This change provides a structured way to present examples, making it easier for developers to learn and integrate web components. Highlights
Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a new code-preview layout, a great feature for showcasing web component examples with a side-by-side view of the code and rendered output. However, a security audit identified potential Cross-Site Scripting (XSS) vulnerabilities due to Liquid template variables being rendered without proper escaping or sanitization. Specifically, the page.iframe_content_url variable is vulnerable to script breakout and javascript: URI injection, and page.title lacks escaping in the <title> tag. These issues should be addressed by applying appropriate Liquid filters like escape and jsonify to prevent XSS.
_layouts/code-preview.html
Outdated
| </div> | ||
| <div id="preview"> | ||
| <h3>Preview</h3> | ||
| <iframe id="iframe" src="{{ page.iframe_content_url }}"></iframe> |
There was a problem hiding this comment.
The iframe's src attribute is populated directly from page.iframe_content_url. If an attacker can control this variable, they can use a javascript: URI to execute arbitrary code in the context of the page. While the escape filter prevents HTML injection, it does not block javascript: URIs. Consider adding a check to ensure the URL starts with a safe prefix like / or https://. Additionally, for security hardening, it's a good practice to add the sandbox="allow-scripts" attribute to restrict actions the content within the iframe can perform, reducing the impact of potential issues.
<iframe id="iframe" src="{{ page.iframe_content_url | escape }}" sandbox="allow-scripts"></iframe>
There was a problem hiding this comment.
A follow-up (and complication) from the above suggestion: the allow-javascript is locking the iframe down heavily (normally exactly what we would want) but it is also by default turning off the allow-popups option, which has a side-effect of preventing clicks on the links like the sources (because they open in a new tab).
Since we completely control the content inside the iframe, and this content only relies on DC code, we could add allow-popups to allow the links to work.
There was a problem hiding this comment.
One last comment that I added before in here (it was hidden because it was in a resolved conversation)!
_layouts/code-preview.html
Outdated
| </div> | ||
| <div id="preview"> | ||
| <h3>Preview</h3> | ||
| <iframe id="iframe" src="{{ page.iframe_content_url }}"></iframe> |
There was a problem hiding this comment.
A follow-up (and complication) from the above suggestion: the allow-javascript is locking the iframe down heavily (normally exactly what we would want) but it is also by default turning off the allow-popups option, which has a side-effect of preventing clicks on the links like the sources (because they open in a new tab).
Since we completely control the content inside the iframe, and this content only relies on DC code, we could add allow-popups to allow the links to work.
nick-nlb
left a comment
nick-nlb
left a comment
There was a problem hiding this comment.
Just the one last comment that was hidden (a follow-up to the bot suggestion), and then LGTM!
Great callout, fixed! |
2 participants
Adds a new code-preview template type that shows source code and a rendered HTML example webpage side-byside.
Part 3 in a series of PRs implementing the changes described in #678.
Changes Made
This PR:
code-preview.htmllayoutTo view the new code-preview layout example:
Screenshots
Desktop
Mobile