Linter: Implement html-require-script-nonce rule
#1384
+303
−5
There are no files selected for viewing
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
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,107 @@ | ||
| # Linter Rule: Require nonce attribute on script tags and helpers | ||
|
|
||
| **Rule:** `html-require-script-nonce` | ||
|
|
||
| ## Description | ||
|
|
||
| Require a `nonce` attribute on inline `<script>` tags, Rails JavaScript helper methods (`javascript_tag`, `javascript_include_tag`) and tag helpers (`tag.script`). This helps enforce a Content Security Policy (CSP) that mitigates cross-site scripting (XSS) attacks. | ||
|
|
||
| ## Rationale | ||
|
|
||
| A Content Security Policy with a nonce-based approach ensures that only scripts with a valid, server-generated nonce are executed by the browser. Without a nonce, inline scripts and dynamically included scripts may be blocked by CSP, or worse, CSP may need to be relaxed with `unsafe-inline`, defeating its purpose. | ||
|
|
||
| Adding `nonce` attributes to all script tags and helpers ensures: | ||
|
|
||
| - Scripts are allowed by the CSP without weakening it | ||
| - Protection against XSS attacks that attempt to inject unauthorized scripts | ||
| - Consistent security practices across the codebase | ||
|
|
||
| ## Examples | ||
|
|
||
| ### ✅ Good | ||
|
|
||
| HTML script tags with a nonce: | ||
|
|
||
| ```erb | ||
| <script nonce="<%= request.content_security_policy_nonce %>"> | ||
| alert("Hello, world!") | ||
| </script> | ||
| ``` | ||
|
|
||
| ```erb | ||
| <script type="text/javascript" nonce="<%= request.content_security_policy_nonce %>"> | ||
| console.log("Hello") | ||
| </script> | ||
| ``` | ||
|
|
||
| Rails helpers with `nonce: true`: | ||
|
|
||
| ```erb | ||
| <%= javascript_tag nonce: true do %> | ||
| alert("Hello, world!") | ||
| <% end %> | ||
| ``` | ||
|
|
||
| ```erb | ||
| <%= javascript_include_tag "application", nonce: true %> | ||
| ``` | ||
|
|
||
| ```erb | ||
| <%= tag.script nonce: true do %> | ||
| alert("Hello, world!") | ||
| <% end %> | ||
| ``` | ||
|
|
||
| Non-JavaScript script types (not flagged): | ||
|
|
||
| ```erb | ||
| <script type="application/json"> | ||
| {"key": "value"} | ||
| </script> | ||
| ``` | ||
|
|
||
| ```erb | ||
| <script type="application/ld+json"> | ||
| {"@context": "https://schema.org"} | ||
| </script> | ||
| ``` | ||
|
|
||
| ### 🚫 Bad | ||
|
|
||
| HTML script tags without a nonce: | ||
|
|
||
| ```erb | ||
| <script> | ||
| alert("Hello, world!") | ||
| </script> | ||
| ``` | ||
|
|
||
| ```erb | ||
| <script type="text/javascript"> | ||
| console.log("Hello") | ||
| </script> | ||
| ``` | ||
|
|
||
| Rails helpers without `nonce: true`: | ||
|
|
||
| ```erb | ||
| <%= javascript_tag do %> | ||
| alert("Hello, world!") | ||
| <% end %> | ||
| ``` | ||
|
|
||
| ```erb | ||
| <%= javascript_include_tag "application" %> | ||
| ``` | ||
|
|
||
| ```erb | ||
| <%= tag.script do %> | ||
| alert("Hello, world!") | ||
| <% end %> | ||
| ``` | ||
|
|
||
| ## References | ||
|
|
||
| - [Inspiration: ERB Lint `RequireScriptNonce` rule](https://github.com/Shopify/erb_lint/blob/main/lib/erb_lint/linters/require_script_nonce.rb) | ||
| - [MDN: Content Security Policy (CSP)](https://developer.mozilla.org/en-US/docs/Web/HTTP/CSP) | ||
| - [Rails: `content_security_policy_nonce`](https://api.rubyonrails.org/classes/ActionDispatch/ContentSecurityPolicy/Request.html) |
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
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,72 @@ | ||
| import { ParserRule } from "../types.js" | ||
| import { BaseRuleVisitor } from "./rule-utils.js" | ||
| import { getTagLocalName, getAttribute, getStaticAttributeValue, hasAttributeValue, findAttributeByName, isERBOpenTagNode } from "@herb-tools/core" | ||
|
|
||
| import type { UnboundLintOffense, LintContext, FullRuleConfig } from "../types.js" | ||
| import type { ParseResult, ParserOptions, HTMLElementNode } from "@herb-tools/core" | ||
|
|
||
| class RequireScriptNonceVisitor extends BaseRuleVisitor { | ||
| visitHTMLElementNode(node: HTMLElementNode): void { | ||
| if (getTagLocalName(node) === "script") { | ||
| this.checkScriptNonce(node) | ||
| } | ||
|
|
||
| super.visitHTMLElementNode(node) | ||
| } | ||
|
|
||
| private checkScriptNonce(node: HTMLElementNode): void { | ||
| if (!this.isJavaScriptTag(node)) return | ||
|
|
||
| const nonceAttribute = this.findAttribute(node, "nonce") | ||
|
|
||
| if (!nonceAttribute || !hasAttributeValue(nonceAttribute)) { | ||
| this.addOffense( | ||
| "Missing a `nonce` attribute on `<script>` tag. Use `request.content_security_policy_nonce`.", | ||
| node.tag_name!.location, | ||
| ) | ||
| } | ||
| } | ||
|
|
||
| private isJavaScriptTag(node: HTMLElementNode): boolean { | ||
| const typeAttribute = this.findAttribute(node, "type") | ||
| if (!typeAttribute) return true | ||
|
|
||
| const typeValue = getStaticAttributeValue(typeAttribute) | ||
| if (typeValue === null) return true | ||
|
|
||
| return typeValue === "text/javascript" || typeValue === "application/javascript" | ||
| } | ||
|
|
||
| private findAttribute(node: HTMLElementNode, name: string) { | ||
| if (isERBOpenTagNode(node.open_tag)) { | ||
| return findAttributeByName(node.open_tag.children, name) | ||
| } | ||
|
|
||
| return getAttribute(node, name) | ||
| } | ||
| } | ||
|
|
||
| export class HTMLRequireScriptNonceRule extends ParserRule { | ||
| static ruleName = "html-require-script-nonce" | ||
|
|
||
| get defaultConfig(): FullRuleConfig { | ||
| return { | ||
| enabled: true, | ||
| severity: "error" | ||
|
Contributor
Author
There was a problem hiding this comment. what's the correct severity?
Owner
There was a problem hiding this comment. We still haven't really balanced all the severities yet. So I think we can keep this one as |
||
| } | ||
| } | ||
|
|
||
| get parserOptions(): Partial<ParserOptions> { | ||
| return { | ||
| action_view_helpers: true, | ||
| } | ||
| } | ||
|
|
||
| check(result: ParseResult, context?: Partial<LintContext>): UnboundLintOffense[] { | ||
| const visitor = new RequireScriptNonceVisitor(this.ruleName, context) | ||
|
|
||
| visitor.visit(result.value) | ||
|
|
||
| return visitor.offenses | ||
| } | ||
| } | ||
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
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,115 @@ | ||
| import dedent from "dedent" | ||
| import { describe, test } from "vitest" | ||
| import { HTMLRequireScriptNonceRule } from "../../src/rules/html-require-script-nonce.js" | ||
| import { createLinterTest } from "../helpers/linter-test-helper.js" | ||
|
|
||
| const { expectNoOffenses, expectError, assertOffenses } = createLinterTest(HTMLRequireScriptNonceRule) | ||
|
|
||
| describe("html-require-script-nonce", () => { | ||
| describe("HTML script tags", () => { | ||
| test("passes when nonce attribute is present with a value", () => { | ||
| expectNoOffenses('<script nonce="abc123"></script>') | ||
| }) | ||
|
|
||
| test("passes when nonce attribute is present with ERB value", () => { | ||
| expectNoOffenses('<script nonce="<%= request.content_security_policy_nonce %>"></script>') | ||
| }) | ||
|
|
||
| test("fails when nonce attribute is missing", () => { | ||
| expectError("Missing a `nonce` attribute on `<script>` tag. Use `request.content_security_policy_nonce`.") | ||
|
|
||
| assertOffenses("<script></script>") | ||
| }) | ||
|
|
||
| test("fails when nonce attribute has no value", () => { | ||
| expectError("Missing a `nonce` attribute on `<script>` tag. Use `request.content_security_policy_nonce`.") | ||
|
|
||
| assertOffenses("<script nonce></script>") | ||
| }) | ||
|
|
||
| test("fails when type is text/javascript and nonce is missing", () => { | ||
| expectError("Missing a `nonce` attribute on `<script>` tag. Use `request.content_security_policy_nonce`.") | ||
|
|
||
| assertOffenses('<script type="text/javascript"></script>') | ||
| }) | ||
|
|
||
| test("fails when type is application/javascript and nonce is missing", () => { | ||
| expectError("Missing a `nonce` attribute on `<script>` tag. Use `request.content_security_policy_nonce`.") | ||
|
|
||
| assertOffenses('<script type="application/javascript"></script>') | ||
| }) | ||
|
|
||
| test("passes when type is text/javascript and nonce is present", () => { | ||
| expectNoOffenses('<script type="text/javascript" nonce="abc123"></script>') | ||
| }) | ||
|
|
||
| test("passes when type is application/javascript and nonce is present", () => { | ||
| expectNoOffenses('<script type="application/javascript" nonce="abc123"></script>') | ||
| }) | ||
|
|
||
| test("passes when type is not JavaScript", () => { | ||
| expectNoOffenses('<script type="application/json"></script>') | ||
| }) | ||
|
|
||
| test("passes when type is application/ld+json", () => { | ||
| expectNoOffenses('<script type="application/ld+json">{"@context": "https://schema.org"}</script>') | ||
| }) | ||
|
|
||
| test("ignores non-script tags", () => { | ||
| expectNoOffenses('<div nonce="abc123"></div>') | ||
| }) | ||
| }) | ||
|
|
||
| describe("ERB javascript helpers", () => { | ||
| test("fails when javascript_tag is used without nonce", () => { | ||
| expectError("Missing a `nonce` attribute on `<script>` tag. Use `request.content_security_policy_nonce`.") | ||
|
|
||
| assertOffenses(dedent` | ||
| <%= javascript_tag %> | ||
| `) | ||
| }) | ||
|
|
||
| test("fails when javascript_include_tag is used without nonce", () => { | ||
| expectError("Missing a `nonce` attribute on `<script>` tag. Use `request.content_security_policy_nonce`.") | ||
|
|
||
| assertOffenses(dedent` | ||
| <%= javascript_include_tag "script" %> | ||
| `) | ||
| }) | ||
|
|
||
| test("passes when javascript_tag is used with nonce", () => { | ||
| expectNoOffenses(dedent` | ||
| <%= javascript_tag nonce: true %> | ||
| `) | ||
| }) | ||
|
|
||
| test("passes when javascript_include_tag is used with nonce", () => { | ||
| expectNoOffenses(dedent` | ||
| <%= javascript_include_tag "script", nonce: true %> | ||
| `) | ||
| }) | ||
|
|
||
| }) | ||
|
|
||
| describe("tag.script helper", () => { | ||
| test("fails when tag.script is used without nonce", () => { | ||
| expectError("Missing a `nonce` attribute on `<script>` tag. Use `request.content_security_policy_nonce`.") | ||
|
|
||
| assertOffenses(dedent` | ||
| <%= tag.script %> | ||
| `) | ||
| }) | ||
|
|
||
| test("passes when tag.script is used with nonce", () => { | ||
| expectNoOffenses(dedent` | ||
| <%= tag.script nonce: true %> | ||
| `) | ||
| }) | ||
| }) | ||
|
|
||
| test("passes using unrelated content_tag", () => { | ||
| expectNoOffenses(dedent` | ||
| <%= content_tag :div, "hello" %> | ||
| `) | ||
| }) | ||
| }) |
Oops, something went wrong.
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.
Uh oh!
There was an error while loading. Please reload this page.
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.
just noticed it's not enabled by default in erb_lint. do we want the same here?
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 its fine to keep this one enabled for now 🙌🏼