Skip to content

Commit 14743fe

Browse files
markokajzermarkokajzer
committed
Linter: Implement html-disallow-inline-scripts rule
1 parent a9b0ecc commit 14743fe

File tree

9 files changed

+385
-53
lines changed

9 files changed

+385
-53
lines changed

‎javascript/packages/linter/docs/rules/README.md‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -83,6 +83,7 @@ This page contains documentation for all Herb Linter rules.
8383
- [`html-body-only-elements`](./html-body-only-elements.md) - Require content elements inside `<body>`.
8484
- [`html-boolean-attributes-no-value`](./html-boolean-attributes-no-value.md) - Prevents values on boolean attributes
8585
- [`html-details-has-summary`](./html-details-has-summary.md) - Require `<summary>` in `<details>` elements
86+
- [`html-disallow-inline-scripts`](./html-disallow-inline-scripts.md) - Disallow inline `<script>` tags and event handler attributes
8687
- [`html-head-only-elements`](./html-head-only-elements.md) - Require head-scoped elements inside `<head>`.
8788
- [`html-iframe-has-title`](./html-iframe-has-title.md) - `iframe` elements must have a `title` attribute
8889
- [`html-img-require-alt`](./html-img-require-alt.md) - Requires `alt` attributes on `<img>` tags
@@ -99,11 +100,11 @@ This page contains documentation for all Herb Linter rules.
99100
- [`html-no-nested-links`](./html-no-nested-links.md) - Prevents nested anchor tags
100101
- [`html-no-positive-tab-index`](./html-no-positive-tab-index.md) - Avoid positive `tabindex` values
101102
- [`html-no-self-closing`](./html-no-self-closing.md) - Disallow self closing tags
102-
- [`html-no-unescaped-entities`](./html-no-unescaped-entities.md) - Disallow unescaped HTML entities
103-
- [`html-no-unknown-tag`](./html-no-unknown-tag.md) - Disallow unknown HTML tags
104103
- [`html-no-space-in-tag`](./html-no-space-in-tag.md) - Disallow spaces in HTML tags
105104
- [`html-no-title-attribute`](./html-no-title-attribute.md) - Avoid using the `title` attribute
106105
- [`html-no-underscores-in-attribute-names`](./html-no-underscores-in-attribute-names.md) - Disallow underscores in HTML attribute names
106+
- [`html-no-unescaped-entities`](./html-no-unescaped-entities.md) - Disallow unescaped HTML entities
107+
- [`html-no-unknown-tag`](./html-no-unknown-tag.md) - Disallow unknown HTML tags
107108
- [`html-require-script-nonce`](./html-require-script-nonce.md) - Require `nonce` attribute on script tags and helpers
108109
- [`html-tag-name-lowercase`](./html-tag-name-lowercase.md) - Enforces lowercase tag names in HTML
109110

‎javascript/packages/linter/docs/rules/html-disallow-inline-scripts.md‎

Lines changed: 85 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,85 @@
1+
# Linter Rule: Disallow inline script tags and event handler attributes
2+
3+
**Rule:** `html-disallow-inline-scripts`
4+
5+
## Description
6+
7+
Disallow the use of inline `<script>` tags and inline JavaScript event handler attributes (e.g. `onclick`, `onload`) in HTML templates.
8+
9+
## Rationale
10+
11+
Inline JavaScript poses a significant security risk and is incompatible with strict Content Security Policy (CSP) configurations (`script-src 'self'`).
12+
13+
All JavaScript should be included via external assets to support strong CSP policies that prevent cross-site scripting (XSS) attacks.
14+
15+
This rule enforces:
16+
17+
- No `<script>` tags embedded directly in templates.
18+
- No event handler attributes (`onclick`, `onmouseover`, etc.).
19+
20+
## Examples
21+
22+
### ✅ Good
23+
24+
```erb
25+
<%= javascript_include_tag "application" %>
26+
```
27+
28+
```erb
29+
<button type="submit" class="btn">Submit</button>
30+
```
31+
32+
```erb
33+
<div data-controller="hello" data-action="click->hello#greet">
34+
Content
35+
</div>
36+
```
37+
38+
```erb
39+
<script type="application/json">
40+
{"key": "value"}
41+
</script>
42+
```
43+
44+
```erb
45+
<script type="application/ld+json">
46+
{"@context": "https://schema.org"}
47+
</script>
48+
```
49+
50+
### 🚫 Bad
51+
52+
```erb
53+
<script>
54+
alert("Hello, world!")
55+
</script>
56+
```
57+
58+
```erb
59+
<script type="text/javascript">
60+
console.log("Hello")
61+
</script>
62+
```
63+
64+
```erb
65+
<button onclick="doSomething()">Click</button>
66+
```
67+
68+
```erb
69+
<body onload="init()"></body>
70+
```
71+
72+
```erb
73+
<div onmouseover="highlight()">Hover me</div>
74+
```
75+
76+
```erb
77+
<form onsubmit="validate()"></form>
78+
```
79+
80+
## References
81+
82+
- Inspired by [@pushcx](https://bsky.app/profile/push.cx/post/3lsfddauapk2o)
83+
- [MDN: Content Security Policy (CSP)](https://developer.mozilla.org/en-US/docs/Web/HTTP/CSP)
84+
- [wooorm/html-event-attributes](https://github.com/wooorm/html-event-attributes)
85+
- [GeeksforGeeks: HTML Event Attributes](https://www.geeksforgeeks.org/html/html-event-attributes/)

‎javascript/packages/linter/src/rules.ts‎

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,7 @@ import { HTMLAvoidBothDisabledAndAriaDisabledRule } from "./rules/html-avoid-bot
6767
import { HTMLBodyOnlyElementsRule } from "./rules/html-body-only-elements.js"
6868
import { HTMLBooleanAttributesNoValueRule } from "./rules/html-boolean-attributes-no-value.js"
6969
import { HTMLDetailsHasSummaryRule } from "./rules/html-details-has-summary.js"
70+
import { HTMLDisallowInlineScriptsRule } from "./rules/html-disallow-inline-scripts.js"
7071
import { HTMLHeadOnlyElementsRule } from "./rules/html-head-only-elements.js"
7172
import { HTMLIframeHasTitleRule } from "./rules/html-iframe-has-title.js"
7273
import { HTMLImgRequireAltRule } from "./rules/html-img-require-alt.js"
@@ -84,11 +85,11 @@ import { HTMLNoEmptyHeadingsRule } from "./rules/html-no-empty-headings.js"
8485
import { HTMLNoNestedLinksRule } from "./rules/html-no-nested-links.js"
8586
import { HTMLNoPositiveTabIndexRule } from "./rules/html-no-positive-tab-index.js"
8687
import { HTMLNoSelfClosingRule } from "./rules/html-no-self-closing.js"
87-
import { HTMLNoUnescapedEntitiesRule } from "./rules/html-no-unescaped-entities.js"
88-
import { HTMLNoUnknownTagRule } from "./rules/html-no-unknown-tag.js"
8988
import { HTMLNoSpaceInTagRule } from "./rules/html-no-space-in-tag.js"
9089
import { HTMLNoTitleAttributeRule } from "./rules/html-no-title-attribute.js"
9190
import { HTMLNoUnderscoresInAttributeNamesRule } from "./rules/html-no-underscores-in-attribute-names.js"
91+
import { HTMLNoUnescapedEntitiesRule } from "./rules/html-no-unescaped-entities.js"
92+
import { HTMLNoUnknownTagRule } from "./rules/html-no-unknown-tag.js"
9293
import { HTMLRequireClosingTagsRule } from "./rules/html-require-closing-tags.js"
9394
import { HTMLRequireScriptNonceRule } from "./rules/html-require-script-nonce.js"
9495
import { HTMLTagNameLowercaseRule } from "./rules/html-tag-name-lowercase.js"
@@ -169,6 +170,7 @@ export const rules: RuleClass[] = [
169170
HTMLBodyOnlyElementsRule,
170171
HTMLBooleanAttributesNoValueRule,
171172
HTMLDetailsHasSummaryRule,
173+
HTMLDisallowInlineScriptsRule,
172174
HTMLHeadOnlyElementsRule,
173175
HTMLIframeHasTitleRule,
174176
HTMLImgRequireAltRule,
@@ -186,11 +188,11 @@ export const rules: RuleClass[] = [
186188
HTMLNoNestedLinksRule,
187189
HTMLNoPositiveTabIndexRule,
188190
HTMLNoSelfClosingRule,
189-
HTMLNoUnescapedEntitiesRule,
190-
HTMLNoUnknownTagRule,
191191
HTMLNoSpaceInTagRule,
192192
HTMLNoTitleAttributeRule,
193193
HTMLNoUnderscoresInAttributeNamesRule,
194+
HTMLNoUnescapedEntitiesRule,
195+
HTMLNoUnknownTagRule,
194196
HTMLRequireClosingTagsRule,
195197
HTMLRequireScriptNonceRule,
196198
HTMLTagNameLowercaseRule,

‎javascript/packages/linter/src/rules/html-disallow-inline-scripts.ts‎

Lines changed: 165 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,165 @@
1+
import { ParserRule } from "../types.js"
2+
import { BaseRuleVisitor, isJavaScriptTagElement } from "./rule-utils.js"
3+
import { getTagLocalName, getAttributeName } from "@herb-tools/core"
4+
5+
import type { UnboundLintOffense, LintContext, FullRuleConfig } from "../types.js"
6+
import type { ParseResult, HTMLElementNode, HTMLAttributeNode } from "@herb-tools/core"
7+
8+
const HTML_EVENT_ATTRIBUTES = new Set([
9+
// Window Event Attributes
10+
"onafterprint",
11+
"onbeforeprint",
12+
"onbeforeunload",
13+
"onerror",
14+
"onhashchange",
15+
"onlanguagechange",
16+
"onload",
17+
"onmessage",
18+
"onmessageerror",
19+
"onoffline",
20+
"ononline",
21+
"onpagehide",
22+
"onpageshow",
23+
"onpopstate",
24+
"onrejectionhandled",
25+
"onresize",
26+
"onstorage",
27+
"onunhandledrejection",
28+
"onunload",
29+
30+
// Form Event Attributes
31+
"onblur",
32+
"onchange",
33+
"onfocus",
34+
"onformdata",
35+
"oninput",
36+
"oninvalid",
37+
"onreset",
38+
"onsearch",
39+
"onselect",
40+
"onsubmit",
41+
42+
// Keyboard Event Attributes
43+
"onkeydown",
44+
"onkeypress",
45+
"onkeyup",
46+
47+
// Mouse Event Attributes
48+
"onauxclick",
49+
"onclick",
50+
"oncontextmenu",
51+
"ondblclick",
52+
"onmousedown",
53+
"onmouseenter",
54+
"onmouseleave",
55+
"onmousemove",
56+
"onmouseout",
57+
"onmouseover",
58+
"onmouseup",
59+
"onwheel",
60+
61+
// Drag Event Attributes
62+
"ondrag",
63+
"ondragend",
64+
"ondragenter",
65+
"ondragleave",
66+
"ondragover",
67+
"ondragstart",
68+
"ondrop",
69+
70+
// Clipboard Event Attributes
71+
"oncopy",
72+
"oncut",
73+
"onpaste",
74+
75+
// Media Event Attributes
76+
"onabort",
77+
"oncanplay",
78+
"oncanplaythrough",
79+
"oncuechange",
80+
"ondurationchange",
81+
"onemptied",
82+
"onended",
83+
"onloadeddata",
84+
"onloadedmetadata",
85+
"onloadstart",
86+
"onpause",
87+
"onplay",
88+
"onplaying",
89+
"onprogress",
90+
"onratechange",
91+
"onseeked",
92+
"onseeking",
93+
"onstalled",
94+
"onsuspend",
95+
"ontimeupdate",
96+
"onvolumechange",
97+
"onwaiting",
98+
99+
// Scroll Event Attributes
100+
"onscroll",
101+
"onscrollend",
102+
103+
// Misc Event Attributes
104+
"onbeforematch",
105+
"onbeforetoggle",
106+
"oncancel",
107+
"onclose",
108+
"oncontextlost",
109+
"oncontextrestored",
110+
"onsecuritypolicyviolation",
111+
"onslotchange",
112+
"ontoggle",
113+
])
114+
115+
class HTMLDisallowInlineScriptsVisitor extends BaseRuleVisitor {
116+
visitHTMLElementNode(node: HTMLElementNode): void {
117+
if (getTagLocalName(node) === "script") {
118+
this.checkInlineScript(node)
119+
}
120+
121+
super.visitHTMLElementNode(node)
122+
}
123+
124+
visitHTMLAttributeNode(node: HTMLAttributeNode): void {
125+
const attributeName = getAttributeName(node)
126+
127+
if (attributeName && HTML_EVENT_ATTRIBUTES.has(attributeName.toLowerCase())) {
128+
this.addOffense(
129+
`Avoid inline event handler \`${attributeName}\`. Use external JavaScript with \`addEventListener\` instead.`,
130+
node.location,
131+
)
132+
}
133+
134+
super.visitHTMLAttributeNode(node)
135+
}
136+
137+
private checkInlineScript(node: HTMLElementNode): void {
138+
if (!isJavaScriptTagElement(node)) return
139+
140+
this.addOffense(
141+
"Avoid inline `<script>` tags. Use `javascript_include_tag` to include external JavaScript files instead.",
142+
node.open_tag!.location,
143+
)
144+
}
145+
}
146+
147+
export class HTMLDisallowInlineScriptsRule extends ParserRule {
148+
static ruleName = "html-disallow-inline-scripts"
149+
static introducedIn = this.version("unreleased")
150+
151+
get defaultConfig(): FullRuleConfig {
152+
return {
153+
enabled: false,
154+
severity: "warning"
155+
}
156+
}
157+
158+
check(result: ParseResult, context?: Partial<LintContext>): UnboundLintOffense[] {
159+
const visitor = new HTMLDisallowInlineScriptsVisitor(this.ruleName, context)
160+
161+
visitor.visit(result.value)
162+
163+
return visitor.offenses
164+
}
165+
}

‎javascript/packages/linter/src/rules/html-require-script-nonce.ts‎

Lines changed: 6 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,9 @@
1-
import { ParserRule } from "../types.js"
2-
import { BaseRuleVisitor } from "./rule-utils.js"
3-
import { getTagLocalName, getAttribute, getStaticAttributeValue, hasAttributeValue, findAttributeByName, isERBOpenTagNode } from "@herb-tools/core"
1+
import { getTagLocalName, getStaticAttributeValue, hasAttributeValue } from "@herb-tools/core"
2+
import type { ParseResult, ParserOptions, HTMLElementNode, HTMLAttributeNode } from "@herb-tools/core"
43

4+
import { BaseRuleVisitor, findElementAttribute, isJavaScriptTagElement } from "./rule-utils.js"
5+
import { ParserRule } from "../types.js"
56
import type { UnboundLintOffense, LintContext, FullRuleConfig } from "../types.js"
6-
import type { ParseResult, ParserOptions, HTMLElementNode, HTMLAttributeNode } from "@herb-tools/core"
77

88
const HELPERS_WITH_CSP_NONCE_SUPPORT = [
99
"ActionView::Helpers::AssetTagHelper#javascript_include_tag",
@@ -20,9 +20,9 @@ class RequireScriptNonceVisitor extends BaseRuleVisitor {
2020
}
2121

2222
private checkScriptNonce(node: HTMLElementNode): void {
23-
if (!this.isJavaScriptTag(node)) return
23+
if (!isJavaScriptTagElement(node)) return
2424

25-
const nonceAttribute = this.findAttribute(node, "nonce")
25+
const nonceAttribute = findElementAttribute(node, "nonce")
2626

2727
if (!nonceAttribute || !hasAttributeValue(nonceAttribute)) {
2828
this.addOffense(
@@ -63,24 +63,6 @@ class RequireScriptNonceVisitor extends BaseRuleVisitor {
6363

6464
return node.element_source ?? "unknown"
6565
}
66-
67-
private isJavaScriptTag(node: HTMLElementNode): boolean {
68-
const typeAttribute = this.findAttribute(node, "type")
69-
if (!typeAttribute) return true
70-
71-
const typeValue = getStaticAttributeValue(typeAttribute)
72-
if (typeValue === null) return true
73-
74-
return typeValue === "text/javascript" || typeValue === "application/javascript"
75-
}
76-
77-
private findAttribute(node: HTMLElementNode, name: string) {
78-
if (isERBOpenTagNode(node.open_tag)) {
79-
return findAttributeByName(node.open_tag.children, name)
80-
}
81-
82-
return getAttribute(node, name)
83-
}
8466
}
8567

8668
export class HTMLRequireScriptNonceRule extends ParserRule {

‎javascript/packages/linter/src/rules/index.ts‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -69,6 +69,7 @@ export * from "./html-avoid-both-disabled-and-aria-disabled.js"
6969
export * from "./html-body-only-elements.js"
7070
export * from "./html-boolean-attributes-no-value.js"
7171
export * from "./html-details-has-summary.js"
72+
export * from "./html-disallow-inline-scripts.js"
7273
export * from "./html-head-only-elements.js"
7374
export * from "./html-iframe-has-title.js"
7475
export * from "./html-img-require-alt.js"
@@ -86,11 +87,11 @@ export * from "./html-no-empty-headings.js"
8687
export * from "./html-no-nested-links.js"
8788
export * from "./html-no-positive-tab-index.js"
8889
export * from "./html-no-self-closing.js"
89-
export * from "./html-no-unescaped-entities.js"
90-
export * from "./html-no-unknown-tag.js"
9190
export * from "./html-no-space-in-tag.js"
9291
export * from "./html-no-title-attribute.js"
9392
export * from "./html-no-underscores-in-attribute-names.js"
93+
export * from "./html-no-unescaped-entities.js"
94+
export * from "./html-no-unknown-tag.js"
9495
export * from "./html-require-closing-tags.js"
9596
export * from "./html-require-script-nonce.js"
9697
export * from "./html-tag-name-lowercase.js"

0 commit comments

Comments
 (0)