From 2705431759a7c7bee9526ccbf843b741fb27e42b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B8rgen=20Aaberg?= Date: Tue, 10 Aug 2021 11:22:19 +0200 Subject: [PATCH] [New] `jsx-no-target-blank`: add `forms` option --- README.md | 5 + docs/rules/jsx-no-target-blank.md | 42 +++++-- lib/rules/jsx-no-target-blank.js | 153 ++++++++++++++++--------- lib/util/linkComponents.js | 19 +++ tests/lib/rules/jsx-no-target-blank.js | 58 ++++++++++ 5 files changed, 216 insertions(+), 61 deletions(-) diff --git a/README.md b/README.md index b50f34d052..ad95c7e922 100644 --- a/README.md +++ b/README.md @@ -64,6 +64,11 @@ You should also specify settings that will be shared across all the plugin rules {"property": "observer", "object": "Mobx"}, {"property": "observer", "object": ""} // sets `object` to whatever value `settings.react.pragma` is set to ], + "formComponents": [ + // Components used as alternatives to
for forms, eg. + "CustomForm", + {"name": "Form", "formAttribute": "endpoint"} + ] "linkComponents": [ // Components used as alternatives to for linking, eg. "Hyperlink", diff --git a/docs/rules/jsx-no-target-blank.md b/docs/rules/jsx-no-target-blank.md index 0ca6129ee2..0f5cfbee92 100644 --- a/docs/rules/jsx-no-target-blank.md +++ b/docs/rules/jsx-no-target-blank.md @@ -5,19 +5,28 @@ This rules requires that you accompany `target='_blank'` attributes with `rel='n ## Rule Details -This rule aims to prevent user generated links from creating security vulnerabilities by requiring `rel='noreferrer'` for external links, and optionally any dynamically generated links. +This rule aims to prevent user generated link hrefs and form actions from creating security vulnerabilities by requiring `rel='noreferrer'` for external link hrefs and form actions, and optionally any dynamically generated link hrefs and form actions. ## Rule Options + ```json ... -"react/jsx-no-target-blank": [, { "allowReferrer": , "enforceDynamicLinks": }] +"react/jsx-no-target-blank": [, { + "allowReferrer": , + "enforceDynamicLinks": , + "links": , + "forms": , +}] ... ``` -* allow-referrer: optional boolean. If `true` does not require `noreferrer`. Defaults to `false`. -* enabled: for enabling the rule. 0=off, 1=warn, 2=error. Defaults to 0. -* enforceDynamicLinks: optional string, 'always' or 'never' -* warnOnSpreadAttributes: optional boolean. Defaults to `false`. +* `allowReferrer`: optional boolean. If `true` does not require `noreferrer`. Defaults to `false`. +* `enabled`: for enabling the rule. 0=off, 1=warn, 2=error. Defaults to 0. +* `enforceDynamicLinks`: optional string, 'always' or 'never' +* `warnOnSpreadAttributes`: optional boolean. Defaults to `false`. +* `enforceDynamicLinks` - enforce: optional string, 'always' or 'never' +* `links` - Prevent usage of unsafe `target='_blank'` inside links, defaults to `true` +* `forms` - Prevent usage of unsafe `target='_blank'` inside forms, defaults to `false` ### `enforceDynamicLinks` @@ -74,6 +83,20 @@ Defaults to false. If false, this rule will ignore all spread attributes. If tru ``` +### `links` / `forms` + +When option `forms` is set to `true`, the following is considered an error: + +```jsx +var Hello =
; +``` + +When option `links` is set to `true`, the following is considered an error: + +```jsx +var Hello = +``` + ### Custom link components This rule supports the ability to use custom components for links, such as `` which is popular in libraries like `react-router`, `next.js` and `gatsby`. To enable this, define your custom link components in the global [shared settings](https://github.com/yannickcr/eslint-plugin-react/blob/master/README.md#configuration) under the `linkComponents` configuration area. Once configured, this rule will check those components as if they were `` elements. @@ -94,9 +117,14 @@ var Hello = var Hello = ``` +### Custom form components + +This rule supports the ability to use custom components for forms. To enable this, define your custom form components in the global [shared settings](https://github.com/yannickcr/eslint-plugin-react/blob/master/README.md#configuration) under the `formComponents` configuration area. Once configured, this rule will check those components as if they were `
` elements. + ## When To Override It + For links to a trusted host (e.g. internal links to your own site, or links to a another host you control, where you can be certain this security vulnerability does not exist), you may want to keep the HTTP Referer header for analytics purposes. ## When Not To Use It -If you do not have any external links, you can disable this rule. +If you do not have any external links or forms, you can disable this rule. diff --git a/lib/rules/jsx-no-target-blank.js b/lib/rules/jsx-no-target-blank.js index 9e3f9d75d4..0d68d75c83 100644 --- a/lib/rules/jsx-no-target-blank.js +++ b/lib/rules/jsx-no-target-blank.js @@ -121,6 +121,14 @@ module.exports = { }, warnOnSpreadAttributes: { type: 'boolean' + }, + links: { + type: 'boolean', + default: true + }, + forms: { + type: 'boolean', + default: false } }, additionalProperties: false @@ -128,80 +136,117 @@ module.exports = { }, create(context) { - const configuration = context.options[0] || {}; + const configuration = Object.assign( + { + links: true, + forms: false + }, + context.options[0] + ); const allowReferrer = configuration.allowReferrer || false; const warnOnSpreadAttributes = configuration.warnOnSpreadAttributes || false; const enforceDynamicLinks = configuration.enforceDynamicLinks || 'always'; - const components = linkComponentsUtil.getLinkComponents(context); + const linkComponents = linkComponentsUtil.getLinkComponents(context); + const formComponents = linkComponentsUtil.getFormComponents(context); return { JSXOpeningElement(node) { - if (!components.has(node.name.name)) { - return; - } - const targetIndex = findLastIndex(node.attributes, (attr) => attr.name && attr.name.name === 'target'); const spreadAttributeIndex = findLastIndex(node.attributes, (attr) => (attr.type === 'JSXSpreadAttribute')); - if (!attributeValuePossiblyBlank(node.attributes[targetIndex])) { - const hasSpread = spreadAttributeIndex >= 0; + if (linkComponents.has(node.name.name)) { + if (!attributeValuePossiblyBlank(node.attributes[targetIndex])) { + const hasSpread = spreadAttributeIndex >= 0; - if (warnOnSpreadAttributes && hasSpread) { - // continue to check below - } else if ((hasSpread && targetIndex < spreadAttributeIndex) || !hasSpread || !warnOnSpreadAttributes) { - return; + if (warnOnSpreadAttributes && hasSpread) { + // continue to check below + } else if ((hasSpread && targetIndex < spreadAttributeIndex) || !hasSpread || !warnOnSpreadAttributes) { + return; + } } - } - const linkAttribute = components.get(node.name.name); - const hasDangerousLink = hasExternalLink(node, linkAttribute, warnOnSpreadAttributes, spreadAttributeIndex) - || (enforceDynamicLinks === 'always' && hasDynamicLink(node, linkAttribute)); - if (hasDangerousLink && !hasSecureRel(node, allowReferrer, warnOnSpreadAttributes, spreadAttributeIndex)) { - context.report({ - node, - messageId: 'noTargetBlank', - fix(fixer) { - // eslint 5 uses `node.attributes`; eslint 6+ uses `node.parent.attributes` - const nodeWithAttrs = node.parent.attributes ? node.parent : node; - // eslint 5 does not provide a `name` property on JSXSpreadElements - const relAttribute = nodeWithAttrs.attributes.find((attr) => attr.name && attr.name.name === 'rel'); - - if (targetIndex < spreadAttributeIndex || (spreadAttributeIndex >= 0 && !relAttribute)) { - return null; - } + const linkAttribute = linkComponents.get(node.name.name); + const hasDangerousLink = hasExternalLink(node, linkAttribute, warnOnSpreadAttributes, spreadAttributeIndex) + || (enforceDynamicLinks === 'always' && hasDynamicLink(node, linkAttribute)); + if (hasDangerousLink && !hasSecureRel(node, allowReferrer, warnOnSpreadAttributes, spreadAttributeIndex)) { + context.report({ + node, + messageId: 'noTargetBlank', + fix(fixer) { + // eslint 5 uses `node.attributes`; eslint 6+ uses `node.parent.attributes` + const nodeWithAttrs = node.parent.attributes ? node.parent : node; + // eslint 5 does not provide a `name` property on JSXSpreadElements + const relAttribute = nodeWithAttrs.attributes.find((attr) => attr.name && attr.name.name === 'rel'); + + if (targetIndex < spreadAttributeIndex || (spreadAttributeIndex >= 0 && !relAttribute)) { + return null; + } - if (!relAttribute) { - return fixer.insertTextAfter(nodeWithAttrs.attributes.slice(-1)[0], ' rel="noreferrer"'); - } + if (!relAttribute) { + return fixer.insertTextAfter(nodeWithAttrs.attributes.slice(-1)[0], ' rel="noreferrer"'); + } - if (!relAttribute.value) { - return fixer.insertTextAfter(relAttribute, '="noreferrer"'); - } + if (!relAttribute.value) { + return fixer.insertTextAfter(relAttribute, '="noreferrer"'); + } - if (relAttribute.value.type === 'Literal') { - const parts = relAttribute.value.value - .split('noreferrer') - .filter(Boolean); - return fixer.replaceText(relAttribute.value, `"${parts.concat('noreferrer').join(' ')}"`); - } + if (relAttribute.value.type === 'Literal') { + const parts = relAttribute.value.value + .split('noreferrer') + .filter(Boolean); + return fixer.replaceText(relAttribute.value, `"${parts.concat('noreferrer').join(' ')}"`); + } - if (relAttribute.value.type === 'JSXExpressionContainer') { - if (relAttribute.value.expression.type === 'Literal') { - if (typeof relAttribute.value.expression.value === 'string') { - const parts = relAttribute.value.expression.value - .split('noreferrer') - .filter(Boolean); - return fixer.replaceText(relAttribute.value.expression, `"${parts.concat('noreferrer').join(' ')}"`); + if (relAttribute.value.type === 'JSXExpressionContainer') { + if (relAttribute.value.expression.type === 'Literal') { + if (typeof relAttribute.value.expression.value === 'string') { + const parts = relAttribute.value.expression.value + .split('noreferrer') + .filter(Boolean); + return fixer.replaceText(relAttribute.value.expression, `"${parts.concat('noreferrer').join(' ')}"`); + } + + // for undefined, boolean, number, symbol, bigint, and null + return fixer.replaceText(relAttribute.value, '"noreferrer"'); } - - // for undefined, boolean, number, symbol, bigint, and null - return fixer.replaceText(relAttribute.value, '"noreferrer"'); } - } - return null; + return null; + } + }); + } + } + if (formComponents.has(node.name.name)) { + if (!attributeValuePossiblyBlank(node.attributes[targetIndex])) { + const hasSpread = spreadAttributeIndex >= 0; + + if (warnOnSpreadAttributes && hasSpread) { + // continue to check below + } else if ( + (hasSpread && targetIndex < spreadAttributeIndex) + || !hasSpread + || !warnOnSpreadAttributes + ) { + return; } - }); + } + + if (!configuration.forms || hasSecureRel(node)) { + return; + } + + const formAttribute = formComponents.get(node.name.name); + + if ( + hasExternalLink(node, formAttribute) + || (enforceDynamicLinks === 'always' + && hasDynamicLink(node, formAttribute)) + ) { + context.report({ + node, + messageId: 'noTargetBlank' + }); + } } } }; diff --git a/lib/util/linkComponents.js b/lib/util/linkComponents.js index 066b2ea249..1f5eb6a358 100644 --- a/lib/util/linkComponents.js +++ b/lib/util/linkComponents.js @@ -9,6 +9,24 @@ const DEFAULT_LINK_COMPONENTS = ['a']; const DEFAULT_LINK_ATTRIBUTE = 'href'; +/** TODO: type {(string | { name: string, formAttribute: string })[]} */ +/** @type {any} */ +const DEFAULT_FORM_COMPONENTS = ['form']; +const DEFAULT_FORM_ATTRIBUTE = 'action'; + +function getFormComponents(context) { + const settings = context.settings || {}; + const formComponents = /** @type {typeof DEFAULT_FORM_COMPONENTS} */ ( + DEFAULT_FORM_COMPONENTS.concat(settings.formComponents || []) + ); + return new Map(formComponents.map((value) => { + if (typeof value === 'string') { + return [value, DEFAULT_FORM_ATTRIBUTE]; + } + return [value.name, value.formAttribute]; + })); +} + function getLinkComponents(context) { const settings = context.settings || {}; const linkComponents = /** @type {typeof DEFAULT_LINK_COMPONENTS} */ ( @@ -23,5 +41,6 @@ function getLinkComponents(context) { } module.exports = { + getFormComponents, getLinkComponents }; diff --git a/tests/lib/rules/jsx-no-target-blank.js b/tests/lib/rules/jsx-no-target-blank.js index ee59331399..22503c5e78 100644 --- a/tests/lib/rules/jsx-no-target-blank.js +++ b/tests/lib/rules/jsx-no-target-blank.js @@ -115,6 +115,26 @@ ruleTester.run('jsx-no-target-blank', rule, { }, { code: '' + }, + { + code: '', + options: [{forms: false}] + }, + { + code: '', + options: [{forms: false, links: true}] + }, + { + code: '
', + options: [] + }, + { + code: '
', + options: [{forms: true}] + }, + { + code: '
', + options: [{forms: true, links: false}] } ], invalid: [ @@ -286,6 +306,44 @@ ruleTester.run('jsx-no-target-blank', rule, { options: [{ warnOnSpreadAttributes: true }] + }, + { + code: '', + output: '', + options: [{links: true}], + errors: defaultErrors + }, + { + code: '', + output: '', + options: [{links: true, forms: true}], + errors: defaultErrors + }, + { + code: '', + output: '', + options: [{links: true, forms: false}], + errors: defaultErrors + }, + { + code: '
', + options: [{forms: true}], + errors: defaultErrors + }, + { + code: '
', + options: [{forms: true}], + errors: defaultErrors + }, + { + code: '
', + options: [{forms: true}], + errors: defaultErrors + }, + { + code: '
', + options: [{forms: true, links: false}], + errors: defaultErrors } ] });