diff --git a/javascript/packages/linter/docs/rules/README.md b/javascript/packages/linter/docs/rules/README.md index a506bccaf..3039a6b8b 100644 --- a/javascript/packages/linter/docs/rules/README.md +++ b/javascript/packages/linter/docs/rules/README.md @@ -97,14 +97,16 @@ This page contains documentation for all Herb Linter rules. - [`html-no-duplicate-ids`](./html-no-duplicate-ids.md) - Prevents duplicate IDs within a document - [`html-no-duplicate-meta-names`](./html-no-duplicate-meta-names.md) - Duplicate `` name attributes are not allowed. - [`html-no-empty-attributes`](./html-no-empty-attributes.md) - Attributes must not have empty values +- [`html-no-event-handlers`](./html-no-event-handlers.md) - Disallow inline event handler attributes - [`html-no-nested-links`](./html-no-nested-links.md) - Prevents nested anchor tags - [`html-no-positive-tab-index`](./html-no-positive-tab-index.md) - Avoid positive `tabindex` values +- [`html-no-script-elements`](./html-no-script-elements.md) - Disallow inline script elements - [`html-no-self-closing`](./html-no-self-closing.md) - Disallow self closing tags -- [`html-no-unescaped-entities`](./html-no-unescaped-entities.md) - Disallow unescaped HTML entities -- [`html-no-unknown-tag`](./html-no-unknown-tag.md) - Disallow unknown HTML tags - [`html-no-space-in-tag`](./html-no-space-in-tag.md) - Disallow spaces in HTML tags - [`html-no-title-attribute`](./html-no-title-attribute.md) - Avoid using the `title` attribute - [`html-no-underscores-in-attribute-names`](./html-no-underscores-in-attribute-names.md) - Disallow underscores in HTML attribute names +- [`html-no-unescaped-entities`](./html-no-unescaped-entities.md) - Disallow unescaped HTML entities +- [`html-no-unknown-tag`](./html-no-unknown-tag.md) - Disallow unknown HTML tags - [`html-require-script-nonce`](./html-require-script-nonce.md) - Require `nonce` attribute on script tags and helpers - [`html-tag-name-lowercase`](./html-tag-name-lowercase.md) - Enforces lowercase tag names in HTML diff --git a/javascript/packages/linter/docs/rules/html-no-event-handlers.md b/javascript/packages/linter/docs/rules/html-no-event-handlers.md new file mode 100644 index 000000000..19d45eeee --- /dev/null +++ b/javascript/packages/linter/docs/rules/html-no-event-handlers.md @@ -0,0 +1,64 @@ +# Linter Rule: Disallow inline event handler attributes + +**Rule:** `html-no-event-handlers` + +## Description + +Disallow the use of inline JavaScript event handler attributes (e.g. `onclick`, `onload`) in HTML templates. + +## Rationale + +Inline JavaScript poses a significant security risk and is incompatible with strict Content Security Policy (CSP) configurations (`script-src 'self'`). + +All JavaScript should be included via external assets to support strong CSP policies that prevent cross-site scripting (XSS) attacks. + +This rule enforces: + +- No event handler attributes (`onclick`, `onmouseover`, etc.) on HTML elements. +- No event handler attributes on ActionView tag helpers (e.g. `<%= tag.button onclick: "..." %>`). + +## Examples + +### ✅ Good + +```erb + +``` + +```erb +
+ Content +
+``` + +```erb +<%= tag.button "Submit", class: "btn" %> +``` + +### 🚫 Bad + +```erb + +``` + +```erb + +``` + +```erb +
Hover me
+``` + +```erb +
+``` + +```erb +<%= tag.button "Submit", onclick: "doSomething()" %> +``` + +## References + +- Inspired by [@pushcx](https://bsky.app/profile/push.cx/post/3lsfddauapk2o) +- [MDN: Content Security Policy (CSP)](https://developer.mozilla.org/en-US/docs/Web/HTTP/CSP) +- [wooorm/html-event-attributes](https://github.com/wooorm/html-event-attributes) diff --git a/javascript/packages/linter/docs/rules/html-no-script-elements.md b/javascript/packages/linter/docs/rules/html-no-script-elements.md new file mode 100644 index 000000000..3b66d4843 --- /dev/null +++ b/javascript/packages/linter/docs/rules/html-no-script-elements.md @@ -0,0 +1,56 @@ +# Linter Rule: Disallow inline script elements + +**Rule:** `html-no-script-elements` + +## Description + +Disallow the use of inline ` +``` + +```erb + +``` + +### 🚫 Bad + +```erb + +``` + +```erb + +``` + +## References + +- Inspired by [@pushcx](https://bsky.app/profile/push.cx/post/3lsfddauapk2o) +- [MDN: Content Security Policy (CSP)](https://developer.mozilla.org/en-US/docs/Web/HTTP/CSP) diff --git a/javascript/packages/linter/src/rules.ts b/javascript/packages/linter/src/rules.ts index 7a4f935b3..f032a9f9d 100644 --- a/javascript/packages/linter/src/rules.ts +++ b/javascript/packages/linter/src/rules.ts @@ -82,14 +82,16 @@ import { HTMLNoDuplicateIdsRule } from "./rules/html-no-duplicate-ids.js" import { HTMLNoDuplicateMetaNamesRule } from "./rules/html-no-duplicate-meta-names.js" import { HTMLNoEmptyAttributesRule } from "./rules/html-no-empty-attributes.js" import { HTMLNoEmptyHeadingsRule } from "./rules/html-no-empty-headings.js" +import { HTMLNoEventHandlersRule } from "./rules/html-no-event-handlers.js" import { HTMLNoNestedLinksRule } from "./rules/html-no-nested-links.js" import { HTMLNoPositiveTabIndexRule } from "./rules/html-no-positive-tab-index.js" +import { HTMLNoScriptElementsRule } from "./rules/html-no-script-elements.js" import { HTMLNoSelfClosingRule } from "./rules/html-no-self-closing.js" -import { HTMLNoUnescapedEntitiesRule } from "./rules/html-no-unescaped-entities.js" -import { HTMLNoUnknownTagRule } from "./rules/html-no-unknown-tag.js" import { HTMLNoSpaceInTagRule } from "./rules/html-no-space-in-tag.js" import { HTMLNoTitleAttributeRule } from "./rules/html-no-title-attribute.js" import { HTMLNoUnderscoresInAttributeNamesRule } from "./rules/html-no-underscores-in-attribute-names.js" +import { HTMLNoUnescapedEntitiesRule } from "./rules/html-no-unescaped-entities.js" +import { HTMLNoUnknownTagRule } from "./rules/html-no-unknown-tag.js" import { HTMLRequireClosingTagsRule } from "./rules/html-require-closing-tags.js" import { HTMLRequireScriptNonceRule } from "./rules/html-require-script-nonce.js" import { HTMLTagNameLowercaseRule } from "./rules/html-tag-name-lowercase.js" @@ -185,14 +187,16 @@ export const rules: RuleClass[] = [ HTMLNoDuplicateMetaNamesRule, HTMLNoEmptyAttributesRule, HTMLNoEmptyHeadingsRule, + HTMLNoEventHandlersRule, HTMLNoNestedLinksRule, HTMLNoPositiveTabIndexRule, + HTMLNoScriptElementsRule, HTMLNoSelfClosingRule, - HTMLNoUnescapedEntitiesRule, - HTMLNoUnknownTagRule, HTMLNoSpaceInTagRule, HTMLNoTitleAttributeRule, HTMLNoUnderscoresInAttributeNamesRule, + HTMLNoUnescapedEntitiesRule, + HTMLNoUnknownTagRule, HTMLRequireClosingTagsRule, HTMLRequireScriptNonceRule, HTMLTagNameLowercaseRule, diff --git a/javascript/packages/linter/src/rules/html-no-event-handlers.ts b/javascript/packages/linter/src/rules/html-no-event-handlers.ts new file mode 100644 index 000000000..fa49fbe3d --- /dev/null +++ b/javascript/packages/linter/src/rules/html-no-event-handlers.ts @@ -0,0 +1,154 @@ +import { getAttributeName } from "@herb-tools/core" +import type { ParseResult, ParserOptions, HTMLAttributeNode } from "@herb-tools/core" + +import { BaseRuleVisitor } from "./rule-utils.js" +import { ParserRule } from "../types.js" +import type { UnboundLintOffense, LintContext, FullRuleConfig } from "../types.js" + +const HTML_EVENT_ATTRIBUTES = new Set([ + // Window Event Attributes + "onafterprint", + "onbeforeprint", + "onbeforeunload", + "onerror", + "onhashchange", + "onlanguagechange", + "onload", + "onmessage", + "onmessageerror", + "onoffline", + "ononline", + "onpagehide", + "onpageshow", + "onpopstate", + "onrejectionhandled", + "onresize", + "onstorage", + "onunhandledrejection", + "onunload", + + // Form Event Attributes + "onblur", + "onchange", + "onfocus", + "onformdata", + "oninput", + "oninvalid", + "onreset", + "onsearch", + "onselect", + "onsubmit", + + // Keyboard Event Attributes + "onkeydown", + "onkeypress", + "onkeyup", + + // Mouse Event Attributes + "onauxclick", + "onclick", + "oncontextmenu", + "ondblclick", + "onmousedown", + "onmouseenter", + "onmouseleave", + "onmousemove", + "onmouseout", + "onmouseover", + "onmouseup", + "onwheel", + + // Drag Event Attributes + "ondrag", + "ondragend", + "ondragenter", + "ondragleave", + "ondragover", + "ondragstart", + "ondrop", + + // Clipboard Event Attributes + "oncopy", + "oncut", + "onpaste", + + // Media Event Attributes + "onabort", + "oncanplay", + "oncanplaythrough", + "oncuechange", + "ondurationchange", + "onemptied", + "onended", + "onloadeddata", + "onloadedmetadata", + "onloadstart", + "onpause", + "onplay", + "onplaying", + "onprogress", + "onratechange", + "onseeked", + "onseeking", + "onstalled", + "onsuspend", + "ontimeupdate", + "onvolumechange", + "onwaiting", + + // Scroll Event Attributes + "onscroll", + "onscrollend", + + // Misc Event Attributes + "onbeforematch", + "onbeforetoggle", + "oncancel", + "onclose", + "oncontextlost", + "oncontextrestored", + "onsecuritypolicyviolation", + "onslotchange", + "ontoggle", +]) + +class HTMLNoEventHandlersVisitor extends BaseRuleVisitor { + visitHTMLAttributeNode(node: HTMLAttributeNode): void { + const attributeName = getAttributeName(node) + + if (attributeName && HTML_EVENT_ATTRIBUTES.has(attributeName.toLowerCase())) { + this.addOffense( + `Avoid inline event handler \`${attributeName}\`. Use external JavaScript with \`addEventListener\` instead or an external library like Stimulus.`, + node.location, + ) + } + + super.visitHTMLAttributeNode(node) + } +} + +export class HTMLNoEventHandlersRule extends ParserRule { + static ruleName = "html-no-event-handlers" + static introducedIn = this.version("unreleased") + + get defaultConfig(): FullRuleConfig { + return { + enabled: false, + severity: "warning" + } + } + + get parserOptions(): Partial { + return { + action_view_helpers: true, + } + } + + check(result: ParseResult, context?: Partial): UnboundLintOffense[] { + const visitor = new HTMLNoEventHandlersVisitor(this.ruleName, context) + + visitor.visit(result.value) + + return visitor.offenses + } +} diff --git a/javascript/packages/linter/src/rules/html-no-script-elements.ts b/javascript/packages/linter/src/rules/html-no-script-elements.ts new file mode 100644 index 000000000..51c6a964d --- /dev/null +++ b/javascript/packages/linter/src/rules/html-no-script-elements.ts @@ -0,0 +1,63 @@ +import { getTagLocalName } from "@herb-tools/core" +import type { ParseResult, ParserOptions, HTMLElementNode } from "@herb-tools/core" + +import { BaseRuleVisitor, isJavaScriptTagElement, findElementAttribute } from "./rule-utils.js" +import { ParserRule } from "../types.js" +import type { UnboundLintOffense, LintContext, FullRuleConfig } from "../types.js" + +const IGNORED_ELEMENT_SOURCES = new Set([ + "ActionView::Helpers::AssetTagHelper#javascript_include_tag", + "ActionView::Helpers::JavaScriptHelper#javascript_tag", +]) + +function isExternalScript(node: HTMLElementNode): boolean { + return !!findElementAttribute(node, "src") && node.body.length === 0 +} + +class HTMLNoScriptElementsVisitor extends BaseRuleVisitor { + visitHTMLElementNode(node: HTMLElementNode): void { + if (this.isInlineScript(node)) { + this.addOffense( + "Avoid inline ` + `) + }) + + test("passes with script tag with external src", () => { + expectNoOffenses(dedent` + + `) + }) + + test("passes with script tag with external src from ERB", () => { + expectNoOffenses(dedent` + + `) + }) + + test("fails with script tag with src and inline body", () => { + expectWarning("Avoid inline ` + `) + }) + + test("fails with empty inline script tag", () => { + expectWarning("Avoid inline `") + }) + + test("fails with inline script tag", () => { + expectWarning("Avoid inline `") + }) + + test("fails with script tag with unallowed type", () => { + expectWarning("Avoid inline ` + `) + }) + + describe("ActionView tag helpers", () => { + test("ignores javascript_tag", () => { + expectNoOffenses(dedent` + <%= javascript_tag do %> + alert("hello") + <% end %> + `) + }) + + test("ignores javascript_include_tag", () => { + expectNoOffenses(dedent` + <%= javascript_include_tag "application" %> + `) + }) + + test("fails with tag.script helper", () => { + expectWarning("Avoid inline `