-
-
Notifications
You must be signed in to change notification settings - Fork 1.1k
feat: implement useIframeSandbox #9949
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
Changes from 1 commit
ff31004
436add6
ec3e97c
d9840fb
c056dad
39188ea
96e91e8
d390106
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,5 @@ | ||
| --- | ||
| "@biomejs/biome": patch | ||
| --- | ||
|
|
||
| Added the nursery rule [`useIframeSandbox`](https://biomejs.dev/linter/rules/use-iframe-sandbox), which enforces the "sandbox" attribute for iframe tags. | ||
|
ematipico marked this conversation as resolved.
Outdated
|
||
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -3,7 +3,7 @@ use biome_analyze::{ | |
| }; | ||
| use biome_console::markup; | ||
| use biome_diagnostics::Severity; | ||
| use biome_html_syntax::AnyHtmlElement; | ||
| use biome_html_syntax::{AnyHtmlElement, HtmlFileSource}; | ||
| use biome_rowan::{AstNode, TextRange}; | ||
| use biome_rule_options::use_iframe_title::UseIframeTitleOptions; | ||
|
|
||
|
|
@@ -59,9 +59,8 @@ impl Rule for UseIframeTitle { | |
|
|
||
| fn run(ctx: &RuleContext<Self>) -> Self::Signals { | ||
| let element = ctx.query(); | ||
| let file_extension = ctx.file_path().extension()?; | ||
|
|
||
| if !is_iframe_element(element, file_extension) { | ||
| if !is_iframe_element(element, ctx) { | ||
| return None; | ||
| } | ||
|
|
||
|
|
@@ -88,17 +87,19 @@ impl Rule for UseIframeTitle { | |
| } | ||
| } | ||
|
|
||
| /// Checks if the element is an iframe element. | ||
| /// | ||
| /// - In `.html` files, matching is case-insensitive. | ||
| /// - In component-based frameworks, only lowercase `iframe` is matched to avoid flagging custom components like `<Iframe>`. | ||
| fn is_iframe_element(element: &AnyHtmlElement, file_extension: &str) -> bool { | ||
| element.name().is_some_and(|token_text| { | ||
| let is_html_file = file_extension == "html"; | ||
| if is_html_file { | ||
| token_text.eq_ignore_ascii_case("iframe") | ||
| } else { | ||
| token_text == "iframe" | ||
| } | ||
| }) | ||
| fn is_iframe_element(element: &AnyHtmlElement, ctx: &RuleContext<UseIframeTitle>) -> bool { | ||
| let Some(element_name) = element.name() else { | ||
| return false; | ||
| }; | ||
|
|
||
| let source_type = ctx.source_type::<HtmlFileSource>(); | ||
|
|
||
| // In HTML files: case-insensitive (IFRAME, Iframe, iframe all match) | ||
| // In component frameworks (Vue, Svelte, Astro): case-sensitive (only "iframe" matches) | ||
| // This means <Iframe> in Vue/Svelte is treated as a component and ignored | ||
| if source_type.is_html() { | ||
| element_name.text().eq_ignore_ascii_case("iframe") | ||
| } else { | ||
| element_name.text() == "iframe" | ||
| } | ||
| } | ||
|
Comment on lines
+90
to
105
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Will extract this function in a different PR, because it saw more components that could use this. But it will be a bit much for this PR |
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,100 @@ | ||
| use biome_analyze::{ | ||
| Ast, Rule, RuleDiagnostic, RuleSource, context::RuleContext, declare_lint_rule, | ||
| }; | ||
| use biome_console::markup; | ||
| use biome_diagnostics::Severity; | ||
| use biome_html_syntax::{AnyHtmlElement, HtmlFileSource}; | ||
| use biome_rowan::AstNode; | ||
| use biome_rule_options::use_iframe_sandbox::UseIframeSandboxOptions; | ||
|
|
||
| declare_lint_rule! { | ||
| /// Enforce the 'sandbox' attribute for 'iframe' elements. | ||
| /// | ||
| /// The sandbox attribute enables an extra set of restrictions for the content in the iframe. | ||
| /// Using the sandbox attribute is considered a good security practice. | ||
| /// | ||
| /// See [the Mozilla docs](https://developer.mozilla.org/en-US/docs/Web/HTML/Reference/Elements/iframe#sandbox) for details. | ||
| /// | ||
| /// ## Examples | ||
| /// | ||
| /// ### Invalid | ||
| /// | ||
| /// ```html,expect_diagnostic | ||
| /// <iframe src="https://example.com"></iframe> | ||
| /// ``` | ||
| /// | ||
| /// ### Valid | ||
| /// | ||
| /// ```html | ||
| /// <iframe src="https://example.com" sandbox="allow-popups"></iframe> | ||
| /// ``` | ||
| /// | ||
| pub UseIframeSandbox { | ||
| version: "next", | ||
| name: "useIframeSandbox", | ||
| language: "html", | ||
| recommended: false, | ||
| severity: Severity::Warning, | ||
| sources: &[RuleSource::EslintReactDom("no-missing-iframe-sandbox").same(), RuleSource::EslintReactXyz("dom-no-missing-iframe-sandbox").same()], | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. kinda unrelated, but do you think we could have a unit test or something to flag rules that don't have the correct rule sources? specifically for this EslintReactXyz plugin. basically, we should be able to flag in a unit test or something:
The reason I'm asking is because things that aren't enforced automatically degrade over time. Something to think about for another PR.
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Was thinking about that, if we can somehow lint that, could you point me into the right direction on where this kind of linting happens?
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I haven't looked at it too closely yet so not precisely. But we would make these assertions through unit tests, not via linting. Similar to how we have a bunch of unit tests that assert certain lists are sorted so binary searches work. We would probably place these unit tests in the
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Alright, will check that out in another PR |
||
| } | ||
| } | ||
|
|
||
| impl Rule for UseIframeSandbox { | ||
| type Query = Ast<AnyHtmlElement>; | ||
| type State = (); | ||
| type Signals = Option<Self::State>; | ||
| type Options = UseIframeSandboxOptions; | ||
|
|
||
| fn run(ctx: &RuleContext<Self>) -> Self::Signals { | ||
| let element = ctx.query(); | ||
|
|
||
| if !is_iframe_element(element, ctx) { | ||
| return None; | ||
| } | ||
|
|
||
| if element | ||
| .find_attribute_by_name("sandbox") | ||
| .is_none_or(|sandbox_attribute| sandbox_attribute.value().is_none()) | ||
| { | ||
| return Some(()); | ||
| } | ||
|
|
||
| None | ||
| } | ||
|
|
||
| fn diagnostic(ctx: &RuleContext<Self>, _state: &Self::State) -> Option<RuleDiagnostic> { | ||
| let node = ctx.query(); | ||
| Some( | ||
| RuleDiagnostic::new( | ||
| rule_category!(), | ||
| node.range(), | ||
| markup! { | ||
| "Iframe is missing "<Emphasis>"sandbox"</Emphasis>" attribute." | ||
|
Netail marked this conversation as resolved.
Outdated
|
||
| } | ||
| ) | ||
| .note(markup! { | ||
| "The sandbox attribute enables an extra set of restrictions for the content in the iframe, protecting against malicious scripts and other security threats." | ||
| }) | ||
| .note(markup! { | ||
| "Provide a "<Emphasis>"sandbox"</Emphasis>" attribute when using iframe elements." | ||
| }), | ||
| ) | ||
| } | ||
| } | ||
|
|
||
| fn is_iframe_element(element: &AnyHtmlElement, ctx: &RuleContext<UseIframeSandbox>) -> bool { | ||
| let Some(element_name) = element.name() else { | ||
| return false; | ||
| }; | ||
|
|
||
| let source_type = ctx.source_type::<HtmlFileSource>(); | ||
|
|
||
| // In HTML files: case-insensitive (IFRAME, Iframe, iframe all match) | ||
| // In component frameworks (Vue, Svelte, Astro): case-sensitive (only "iframe" matches) | ||
| // This means <Iframe> in Vue/Svelte is treated as a component and ignored | ||
| if source_type.is_html() { | ||
| element_name.text().eq_ignore_ascii_case("iframe") | ||
| } else { | ||
| element_name.text() == "iframe" | ||
| } | ||
| } | ||
|
Comment on lines
+79
to
+94
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same |
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,3 @@ | ||
| <!-- should generate diagnostics --> | ||
| <iframe></iframe> | ||
| <iframe sandbox></iframe> | ||
|
coderabbitai[bot] marked this conversation as resolved.
Outdated
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,52 @@ | ||
| --- | ||
| source: crates/biome_html_analyze/tests/spec_tests.rs | ||
| expression: invalid.html | ||
| --- | ||
| # Input | ||
| ```html | ||
| <!-- should generate diagnostics --> | ||
| <iframe></iframe> | ||
| <iframe sandbox></iframe> | ||
|
|
||
| ``` | ||
|
|
||
| # Diagnostics | ||
| ``` | ||
| invalid.html:2:1 lint/nursery/useIframeSandbox ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ | ||
|
|
||
| ! Iframe is missing sandbox attribute. | ||
|
|
||
| 1 │ <!-- should generate diagnostics --> | ||
| > 2 │ <iframe></iframe> | ||
| │ ^^^^^^^^^^^^^^^^^ | ||
| 3 │ <iframe sandbox></iframe> | ||
| 4 │ | ||
|
|
||
| i The sandbox attribute enables an extra set of restrictions for the content in the iframe, protecting against malicious scripts and other security threats. | ||
|
|
||
| i Provide a sandbox attribute when using iframe elements. | ||
|
|
||
| i This rule belongs to the nursery group, which means it is not yet stable and may change in the future. Visit https://biomejs.dev/linter/#nursery for more information. | ||
|
|
||
|
|
||
| ``` | ||
|
|
||
| ``` | ||
| invalid.html:3:1 lint/nursery/useIframeSandbox ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ | ||
|
|
||
| ! Iframe is missing sandbox attribute. | ||
|
|
||
| 1 │ <!-- should generate diagnostics --> | ||
| 2 │ <iframe></iframe> | ||
| > 3 │ <iframe sandbox></iframe> | ||
| │ ^^^^^^^^^^^^^^^^^^^^^^^^^ | ||
| 4 │ | ||
|
|
||
| i The sandbox attribute enables an extra set of restrictions for the content in the iframe, protecting against malicious scripts and other security threats. | ||
|
|
||
| i Provide a sandbox attribute when using iframe elements. | ||
|
|
||
| i This rule belongs to the nursery group, which means it is not yet stable and may change in the future. Visit https://biomejs.dev/linter/#nursery for more information. | ||
|
|
||
|
|
||
| ``` |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,8 @@ | ||
| <!-- should not generate diagnostics --> | ||
| <a></a> | ||
| <span></span> | ||
| <button type="button">Click me</button> | ||
| <iframe sandbox=""></iframe> | ||
| <iframe sandbox="allow-downloads"></iframe> | ||
| <iframe sandbox="allow-downloads allow-scripts"></iframe> | ||
| <iframe sandbox="allow-downloads allow-scripts allow-forms"></iframe> |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,16 @@ | ||
| --- | ||
| source: crates/biome_html_analyze/tests/spec_tests.rs | ||
| expression: valid.html | ||
| --- | ||
| # Input | ||
| ```html | ||
| <!-- should not generate diagnostics --> | ||
| <a></a> | ||
| <span></span> | ||
| <button type="button">Click me</button> | ||
| <iframe sandbox=""></iframe> | ||
| <iframe sandbox="allow-downloads"></iframe> | ||
| <iframe sandbox="allow-downloads allow-scripts"></iframe> | ||
| <iframe sandbox="allow-downloads allow-scripts allow-forms"></iframe> | ||
|
|
||
| ``` |
Uh oh!
There was an error while loading. Please reload this page.