-
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
feat(lint/html): add noAstroSetHtmlDirective #11322
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
base: main
Are you sure you want to change the base?
Changes from all commits
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,11 @@ | ||
| --- | ||
| "@biomejs/biome": patch | ||
| --- | ||
|
|
||
| Added a new nursery rule [`noAstroSetHtmlDirective`](https://biomejs.dev/linter/rules/no-astro-set-html-directive/), which disallows Astro's `set:html` directive because untrusted content can introduce cross-site scripting vulnerabilities. | ||
|
|
||
| For example, the following snippet triggers the rule: | ||
|
|
||
| ```astro | ||
| <div set:html={content} /> | ||
| ``` |
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.
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 | ||||
|---|---|---|---|---|---|---|
| @@ -0,0 +1,75 @@ | ||||||
| use biome_analyze::{ | ||||||
| Ast, Rule, RuleDiagnostic, RuleDomain, RuleSource, context::RuleContext, declare_lint_rule, | ||||||
| }; | ||||||
| use biome_console::markup; | ||||||
| use biome_html_syntax::AstroSetDirective; | ||||||
| use biome_languages::HtmlFileSource; | ||||||
| use biome_rowan::AstNode; | ||||||
| use biome_rule_options::no_astro_set_html_directive::NoAstroSetHtmlDirectiveOptions; | ||||||
|
|
||||||
| declare_lint_rule! { | ||||||
| /// Disallow the use of Astro's `set:html` directive. | ||||||
| /// | ||||||
| /// `set:html` renders HTML without escaping it. Passing untrusted content to the directive can introduce cross-site scripting vulnerabilities. | ||||||
| /// | ||||||
| /// ## Examples | ||||||
| /// | ||||||
| /// ### Invalid | ||||||
| /// | ||||||
| /// ```astro,expect_diagnostic | ||||||
| /// <div set:html={content} /> | ||||||
| /// ``` | ||||||
| /// | ||||||
| /// ### Valid | ||||||
| /// | ||||||
| /// ```astro | ||||||
| /// <div>{content}</div> | ||||||
| /// ``` | ||||||
| /// | ||||||
| /// ## References | ||||||
| /// | ||||||
| /// - [Astro `set:html` directive](https://docs.astro.build/en/reference/directives-reference/#sethtml) | ||||||
| pub NoAstroSetHtmlDirective { | ||||||
| version: "next", | ||||||
| name: "noAstroSetHtmlDirective", | ||||||
| language: "html", | ||||||
| recommended: false, | ||||||
| domains: &[RuleDomain::Astro], | ||||||
| sources: &[RuleSource::EslintAstro("no-set-html-directive").same()], | ||||||
| } | ||||||
| } | ||||||
|
|
||||||
| impl Rule for NoAstroSetHtmlDirective { | ||||||
| type Query = Ast<AstroSetDirective>; | ||||||
| type State = (); | ||||||
| type Signals = Option<Self::State>; | ||||||
| type Options = NoAstroSetHtmlDirectiveOptions; | ||||||
|
|
||||||
| fn run(ctx: &RuleContext<Self>) -> Self::Signals { | ||||||
| if !ctx.source_type::<HtmlFileSource>().is_astro() { | ||||||
| return None; | ||||||
| } | ||||||
|
|
||||||
| let value = ctx.query().value().ok()?; | ||||||
| let name = value.name().ok()?.token_text_trimmed()?; | ||||||
| (name.text() == "html").then_some(()) | ||||||
| } | ||||||
|
|
||||||
| fn diagnostic(ctx: &RuleContext<Self>, _state: &Self::State) -> Option<RuleDiagnostic> { | ||||||
| Some( | ||||||
| RuleDiagnostic::new( | ||||||
| rule_category!(), | ||||||
| ctx.query().range(), | ||||||
| markup! { | ||||||
| "This "<Emphasis>"set:html"</Emphasis>" directive inserts unescaped HTML." | ||||||
|
Member
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.
Suggested change
|
||||||
| }, | ||||||
| ) | ||||||
| .note(markup! { | ||||||
| "Passing untrusted content to "<Emphasis>"set:html"</Emphasis>" can introduce cross-site scripting vulnerabilities." | ||||||
|
Member
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. Remove the "untrusted" part. With this word you're assuming the content is coming from external sources, but sometimes it's not the case |
||||||
| }) | ||||||
| .note(markup! { | ||||||
| "Use a regular Astro expression to render text. If raw HTML is required, sanitize the value before passing it to "<Emphasis>"set:html"</Emphasis>" and suppress this diagnostic with an explanation." | ||||||
|
Member
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'd remove the "sanitised" part for two reasons:
Still, I would like to mention the sanitisation part, but with some references that explain how to do so. We should mention it in the docs too |
||||||
| }), | ||||||
| ) | ||||||
| } | ||||||
| } | ||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,5 @@ | ||
| <!-- should generate diagnostics --> | ||
| <div set:html={content}></div> | ||
| <section set:html="<p>content</p>"></section> | ||
| <aside set:html=`<p>content</p>`></aside> | ||
| <article set:html></article> |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,150 @@ | ||
| --- | ||
| source: crates/biome_html_analyze/tests/spec_tests.rs | ||
| expression: invalid.astro | ||
| --- | ||
| # Input | ||
| ```astro | ||
| <!-- should generate diagnostics --> | ||
| <div set:html={content}></div> | ||
| <section set:html="<p>content</p>"></section> | ||
| <aside set:html=`<p>content</p>`></aside> | ||
| <article set:html></article> | ||
|
|
||
| ``` | ||
|
|
||
| # Diagnostics | ||
| ``` | ||
| invalid.astro:4:17 parse ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ | ||
|
|
||
| × Unexpected character in unquoted attribute value | ||
|
|
||
| 2 │ <div set:html={content}></div> | ||
| 3 │ <section set:html="<p>content</p>"></section> | ||
| > 4 │ <aside set:html=`<p>content</p>`></aside> | ||
| │ ^ | ||
| 5 │ <article set:html></article> | ||
| 6 │ | ||
|
|
||
|
|
||
| ``` | ||
|
|
||
| ``` | ||
| invalid.astro:4:17 parse ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ | ||
|
|
||
| × Unexpected character ``` | ||
|
|
||
| 2 │ <div set:html={content}></div> | ||
| 3 │ <section set:html="<p>content</p>"></section> | ||
| > 4 │ <aside set:html=`<p>content</p>`></aside> | ||
| │ ^ | ||
| 5 │ <article set:html></article> | ||
| 6 │ | ||
|
|
||
|
|
||
| ``` | ||
|
|
||
| ``` | ||
| invalid.astro:4:18 parse ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ | ||
|
|
||
| × Expected an attribute but instead found '<'. | ||
|
|
||
| 2 │ <div set:html={content}></div> | ||
| 3 │ <section set:html="<p>content</p>"></section> | ||
| > 4 │ <aside set:html=`<p>content</p>`></aside> | ||
| │ ^ | ||
| 5 │ <article set:html></article> | ||
| 6 │ | ||
|
|
||
| i Expected an attribute here. | ||
|
|
||
| 2 │ <div set:html={content}></div> | ||
| 3 │ <section set:html="<p>content</p>"></section> | ||
| > 4 │ <aside set:html=`<p>content</p>`></aside> | ||
| │ ^ | ||
| 5 │ <article set:html></article> | ||
| 6 │ | ||
|
|
||
|
|
||
| ``` | ||
|
|
||
| ``` | ||
| invalid.astro:2:6 lint/nursery/noAstroSetHtmlDirective ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ | ||
|
|
||
| i This set:html directive inserts unescaped HTML. | ||
|
|
||
| 1 │ <!-- should generate diagnostics --> | ||
| > 2 │ <div set:html={content}></div> | ||
| │ ^^^^^^^^^^^^^^^^^^ | ||
| 3 │ <section set:html="<p>content</p>"></section> | ||
| 4 │ <aside set:html=`<p>content</p>`></aside> | ||
|
|
||
| i Passing untrusted content to set:html can introduce cross-site scripting vulnerabilities. | ||
|
|
||
| i Use a regular Astro expression to render text. If raw HTML is required, sanitize the value before passing it to set:html and suppress this diagnostic with an explanation. | ||
|
|
||
| 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.astro:3:10 lint/nursery/noAstroSetHtmlDirective ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ | ||
|
|
||
| i This set:html directive inserts unescaped HTML. | ||
|
|
||
| 1 │ <!-- should generate diagnostics --> | ||
| 2 │ <div set:html={content}></div> | ||
| > 3 │ <section set:html="<p>content</p>"></section> | ||
| │ ^^^^^^^^^^^^^^^^^^^^^^^^^ | ||
| 4 │ <aside set:html=`<p>content</p>`></aside> | ||
| 5 │ <article set:html></article> | ||
|
|
||
| i Passing untrusted content to set:html can introduce cross-site scripting vulnerabilities. | ||
|
|
||
| i Use a regular Astro expression to render text. If raw HTML is required, sanitize the value before passing it to set:html and suppress this diagnostic with an explanation. | ||
|
|
||
| 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.astro:4:8 lint/nursery/noAstroSetHtmlDirective ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ | ||
|
|
||
| i This set:html directive inserts unescaped HTML. | ||
|
|
||
| 2 │ <div set:html={content}></div> | ||
| 3 │ <section set:html="<p>content</p>"></section> | ||
| > 4 │ <aside set:html=`<p>content</p>`></aside> | ||
| │ ^^^^^^^^^ | ||
| 5 │ <article set:html></article> | ||
| 6 │ | ||
|
|
||
| i Passing untrusted content to set:html can introduce cross-site scripting vulnerabilities. | ||
|
|
||
| i Use a regular Astro expression to render text. If raw HTML is required, sanitize the value before passing it to set:html and suppress this diagnostic with an explanation. | ||
|
|
||
| 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.astro:5:10 lint/nursery/noAstroSetHtmlDirective ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ | ||
|
|
||
| i This set:html directive inserts unescaped HTML. | ||
|
|
||
| 3 │ <section set:html="<p>content</p>"></section> | ||
| 4 │ <aside set:html=`<p>content</p>`></aside> | ||
| > 5 │ <article set:html></article> | ||
| │ ^^^^^^^^ | ||
| 6 │ | ||
|
|
||
| i Passing untrusted content to set:html can introduce cross-site scripting vulnerabilities. | ||
|
|
||
| i Use a regular Astro expression to render text. If raw HTML is required, sanitize the value before passing it to set:html and suppress this diagnostic with an explanation. | ||
|
|
||
| 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,4 @@ | ||
| <!-- should not generate diagnostics --> | ||
| <div set:text={content}></div> | ||
| <div>{content}</div> | ||
| <div set:HTML={content}></div> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it should be recommended, and have an error severity