Rule fix: no-append-html: Allow passing selectors to some methods

Fixes #326
wikimedia · Jun 20, 2024 · 1f77008 · 1f77008
1 parent bd0218c
commit 1f77008
Show file tree

Hide file tree

Showing 4 changed files with 31 additions and 4 deletions.
diff --git a/docs/rules/no-append-html.md b/docs/rules/no-append-html.md
@@ -2,13 +2,14 @@
 
 # no-append-html
 
-Disallows using [`.append`](https://api.jquery.com/append/)/[`.prepend`](https://api.jquery.com/prepend/)/[`.before`](https://api.jquery.com/before/)/[`.after`](https://api.jquery.com/after/)/[`.replaceWith`](https://api.jquery.com/replaceWith/)/[`.add`](https://api.jquery.com/add/)/[`.appendTo`](https://api.jquery.com/appendTo/)/[`.prependTo`](https://api.jquery.com/prependTo/) to inject HTML, in order to prevent possible XSS bugs.
+Disallows using [`.append`](https://api.jquery.com/append/)/[`.prepend`](https://api.jquery.com/prepend/)/[`.before`](https://api.jquery.com/before/)/[`.after`](https://api.jquery.com/after/)/[`.replaceWith`](https://api.jquery.com/replaceWith/)/[`.add`](https://api.jquery.com/add/)/[`.appendTo`](https://api.jquery.com/appendTo/)/[`.prependTo`](https://api.jquery.com/prependTo/)/[`.insertBefore`](https://api.jquery.com/insertBefore/)/[`.insertAfter`](https://api.jquery.com/insertAfter/) to inject HTML, in order to prevent possible XSS bugs.
 
 ## Rule details
 
 ❌ Examples of **incorrect** code:
 ```js
 $div.append( '<xss>' );
+$div.append( 'unescaped html' );
 $div.prepend( '<xss>' );
 $div.before( '<xss>' );
 $div.after( '<xss>' );
@@ -47,6 +48,12 @@ $div.append( test ? $el1 : '' );
 
 $el = getSomething();
 $div.append( $el );
+
+$div.add( '.foo' );
+$div.appendTo( '.foo' );
+$div.prependTo( '.foo' );
+$div.insertBefore( '.foo' );
+$div.insertAfter( '.foo' );
 ```
 
 ## Resources

diff --git a/src/rules/no-append-html.js b/src/rules/no-append-html.js
@@ -1,7 +1,11 @@
 'use strict';
 
 const utils = require( '../utils.js' );
-const methods = [ 'append', 'prepend', 'before', 'after', 'replaceWith', 'add', 'appendTo', 'prependTo' ];
+// htmlStrings or jQuery collections
+const htmlOrCollectionMethods = [ 'append', 'prepend', 'before', 'after', 'replaceWith' ];
+// htmlStrings, selectors or jQuery collections
+const htmlOrSelectorOrCollectionMethods = [ 'add', 'appendTo', 'prependTo', 'insertBefore', 'insertAfter' ];
+const allMethods = htmlOrCollectionMethods.concat( htmlOrSelectorOrCollectionMethods );
 
 function alljQueryOrEmpty( context, node ) {
 	if ( node.type === 'ConditionalExpression' ) {
@@ -22,7 +26,7 @@ module.exports = {
 	meta: {
 		type: 'suggestion',
 		docs: {
-			description: 'Disallows using ' + methods.map( utils.jQueryCollectionLink ).join( '/' ) +
+			description: 'Disallows using ' + allMethods.map( utils.jQueryCollectionLink ).join( '/' ) +
 			' to inject HTML, in order to prevent possible XSS bugs.'
 		},
 		schema: []
@@ -32,13 +36,18 @@ module.exports = {
 		'CallExpression:exit': ( node ) => {
 			if ( !(
 				node.callee.type === 'MemberExpression' &&
-					methods.includes( node.callee.property.name )
+				allMethods.includes( node.callee.property.name )
 			) ) {
 				return;
 			}
 			if ( node.arguments.every( ( arg ) => alljQueryOrEmpty( context, arg ) ) ) {
 				return;
 			}
+			if ( htmlOrSelectorOrCollectionMethods.includes( node.callee.property.name ) ) {
+				if ( node.arguments.every( ( arg ) => !utils.isHtmlString( arg ) ) ) {
+					return;
+				}
+			}
 
 			if ( utils.isjQuery( context, node.callee ) ) {
 				context.report( {

diff --git a/src/utils.js b/src/utils.js
@@ -544,6 +544,8 @@ function eventShorthandFixer( node, context, fixer ) {
 function allLiteral( node ) {
 	if ( node.type === 'BinaryExpression' ) {
 		return allLiteral( node.left ) && allLiteral( node.right );
+	} else if ( node.type === 'ConditionalExpression' ) {
+		return allLiteral( node.consequent ) && allLiteral( node.alternate );
 	} else {
 		return node.type === 'Literal';
 	}

diff --git a/tests/rules/no-append-html.js b/tests/rules/no-append-html.js
@@ -27,6 +27,11 @@ ruleTester.run( 'no-append-html', rule, {
 		'$div.append(test ? $el1 : undefined)',
 		'$div.append(test ? $el1 : "")',
 		'$el=getSomething();\n$div.append($el);',
+		'$div.add(".foo")',
+		'$div.appendTo(".foo")',
+		'$div.prependTo(".foo")',
+		'$div.insertBefore(".foo")',
+		'$div.insertAfter(".foo")',
 		{
 			code: 'div.append("<xss>")',
 			docgen: false
@@ -37,6 +42,10 @@ ruleTester.run( 'no-append-html', rule, {
 			code: '$div.append("<xss>")',
 			errors: [ error ]
 		},
+		{
+			code: '$div.append("unescaped html")',
+			errors: [ error ]
+		},
 		{
 			code: '$div.prepend("<xss>")',
 			errors: [ error ]