diff --git a/docs/rules/no-insecure-url.md b/docs/rules/no-insecure-url.md index 09f5369..44de33d 100644 --- a/docs/rules/no-insecure-url.md +++ b/docs/rules/no-insecure-url.md @@ -6,9 +6,10 @@ Insecure protocols such as [HTTP](https://en.wikipedia.org/wiki/Hypertext_Transf - [Rule Test](../../tests/lib/rules/no-insecure-url.js) ## Options -This rule comes with two [default lists](../../lib/rules/no-insecure-url.js#L13): +This rule comes with three [default lists](../../lib/rules/no-insecure-url.js#L13): - **blocklist** - a RegEx list of insecure URL patterns. - **exceptions** - a RegEx list of common false positive patterns. For example, HTTP URLs to XML schemas are usually allowed as they are used as identifiers, not for establishing actual network connections. +- **varExceptions** - a RegEx list of false positive patterns which a derivated from the variable name. For example, a variable that is called "insecureURL" which is used to test HTTP explicitly. These lists can be overrided by providing options. @@ -17,7 +18,8 @@ For example, providing these options... : ```javascript "@microsoft/sdl/no-insecure-url": ["error", { "blocklist": ["^(http|ftp):\\/\\/", "^https:\\/\\/www\\.disallow-example\\.com"], - "exceptions": ["^http:\\/\\/schemas\\.microsoft\\.com\\/\\/?.*"] + "exceptions": ["^http:\\/\\/schemas\\.microsoft\\.com\\/\\/?.*"], + "varExceptions": ["insecure?.*"] }] ``` @@ -30,7 +32,11 @@ For example, providing these options... : - `http://schemas.microsoft.com` - `http://schemas.microsoft.com/sharepoint` - `http://schemas.microsoft.com/path/subpath` - - ... + +... and also overrides the internal variable exceptions list, allowing the following declaration name patterns as exceptions.: +- `var insecureURL = "http://..."` +- `var insecureWebsite = "http://..."` +- ... URLs in neither the blocklist nor the exceptions list, are allowed: - `telnet://`... diff --git a/lib/rules/no-insecure-url.js b/lib/rules/no-insecure-url.js index 4e7abee..7486079 100644 --- a/lib/rules/no-insecure-url.js +++ b/lib/rules/no-insecure-url.js @@ -18,14 +18,16 @@ const DEFAULT_EXCEPTIONS = [ // TODO: add more typical false positives such /^http:(\/\/|\\u002f\\u002f)schemas\.microsoft\.com(\/\/|\\u002f\\u002f)?.*/i, /^http:(\/\/|\\u002f\\u002f)schemas\.openxmlformats\.org(\/\/|\\u002f\\u002f)?.*/i, /^http:(\/|\\u002f){2}localhost(:|\/|\\u002f)*/i, - /^http:(\/|\\u002f){2}localhost(:|\/|\\u002f)*/i, /^http:(\/\/)www\.w3\.org\/1999\/xhtml/i, /^http:(\/\/)www\.w3\.org\/2000\/svg/i ]; +const DEFAULT_VARIABLES_EXECEPTIONS = []; + module.exports = { defaultBlocklist: DEFAULT_BLOCKLIST, defaultExceptions: DEFAULT_EXCEPTIONS, + defaultVarExecptions: DEFAULT_VARIABLES_EXECEPTIONS, meta: { type: "suggestion", fixable: "code", @@ -45,6 +47,12 @@ module.exports = { type: "string" } }, + varExceptions: { + type: "array", + items: { + type: "string" + } + }, }, additionalProperties: false } @@ -62,11 +70,22 @@ module.exports = { const options = context.options[0] || {}; const blocklist = (options.blocklist || DEFAULT_BLOCKLIST).map((pattern) => { return new RegExp(pattern, "i"); }); const exceptions = (options.exceptions || DEFAULT_EXCEPTIONS).map((pattern) => { return new RegExp(pattern, "i"); }); + const varExceptions = (options.varExceptions || DEFAULT_VARIABLES_EXECEPTIONS).map((pattern) => { return new RegExp(pattern, "i"); }); function matches(patterns, value) { return patterns.find((re) => { return re.test(value) }) !== undefined; } + function shouldFix( varExceptions,context, node) { + let sourceCode = context.getSourceCode(); + // check variable for unfixable pattern e.g. `var insecureURL = "http://..."` + let text = node.parent + ? sourceCode.getText(node.parent) + : sourceCode.getText(node); + // if no match, fix the line + return !matches(varExceptions,text); + } + return { "Literal"(node) { if (typeof node.value === "string") { @@ -75,11 +94,19 @@ module.exports = { { // Do nothing } - else if (matches(blocklist, node.value) && !matches(exceptions, node.value)) { - context.report({ - node: node, - messageId: "doNotUseInsecureUrl" - }); + else if (matches(blocklist, node.value) && !matches(exceptions, node.value) && shouldFix(varExceptions,context, node)) { + context.report({ + node: node, + messageId: "doNotUseInsecureUrl", + fix(fixer) { + // Only fix if it contains an http url + if (node.value.toLowerCase().includes("http")) { + let fixedString = node.value.replace(/http:/i, "https:"); + //insert an "s" before ":/" to change http:/ to https:/ + return fixer.replaceText(node, JSON.stringify(fixedString)); + } + }, + }); } } }, @@ -88,15 +115,26 @@ module.exports = { const rawStringText = node.value.raw; const cookedStringText = node.value.cooked; - if ((matches(blocklist, rawStringText) && !matches(exceptions, rawStringText)) || - (matches(blocklist, cookedStringText) && !matches(exceptions, cookedStringText))) { - context.report({ - node: node, - messageId: "doNotUseInsecureUrl" - }); - } - } - } + if (shouldFix(varExceptions,context, node) && (matches(blocklist, rawStringText) && !matches(exceptions, rawStringText)) || + (matches(blocklist, cookedStringText) && !matches(exceptions, cookedStringText))) { + context.report({ + node: node, + messageId: "doNotUseInsecureUrl", + fix(fixer) { + // Only fix if it contains an http url + if (node.value.raw.toLowerCase().includes("http")) { + let escapedString = JSON.stringify(context.getSourceCode().getText(node)); + // delete "" that JSON.stringify created and convert to `` string + escapedString = ``+ escapedString.substring(1, escapedString.length-1); + let fixedString = escapedString.replace(/http:/i, "https:"); + //insert an "s" before ":/" to change http:/ to https:/ + return fixer.replaceText(node, fixedString); + } + } + }); + } + } + }, }; }, }; \ No newline at end of file diff --git a/tests/lib/rules/no-insecure-url.js b/tests/lib/rules/no-insecure-url.js index 13b6328..52f7acf 100644 --- a/tests/lib/rules/no-insecure-url.js +++ b/tests/lib/rules/no-insecure-url.js @@ -58,6 +58,15 @@ ruleTester.run(ruleId, rule, { exceptions: ["HTTP:\/\/www\.allow-example\.com\/?.*", "FtP:\/\/www\.allow-file-example\.com", "LdaP:\/\/www\.allow-ldap-example\.com"] }] }, + { // should allow user-provided exceptions for variable name matches, regardless of upper/lower-case + code: ` + var insecureURL = 'http://www.allow-example.com' + var InSeCuReURL = 'ftp://www.allow-example.com/path' + `, + options: [{ + varExceptions: ["insecure?.*"] + }] + }, { // should allow xml namespaces, as they are not accessed by the browser code: ` @@ -94,6 +103,12 @@ ruleTester.run(ruleId, rule, { var y1 = 'ftp://www.file-examples.com' var y2 = 'FTP://www.file-examples.com' `, + output: ` + var x1 = "https://www.examples.com" + var x2 = "https://www.examples.com" + var y1 = 'ftp://www.file-examples.com' + var y2 = 'FTP://www.file-examples.com' + `, errors: [ { messageId: "doNotUseInsecureUrl", line: 2}, { messageId: "doNotUseInsecureUrl", line: 3}, @@ -108,6 +123,12 @@ ruleTester.run(ruleId, rule, { var y1 = \`ftp://www.file-examples.com\` var y2 = \`FTP://www.file-examples.com\` `, + output: ` + var x1 = \`https://www.template-examples.com\` + var x2 = \`https://www.template-examples.com\` + var y1 = \`ftp://www.file-examples.com\` + var y2 = \`FTP://www.file-examples.com\` + `, errors: [ { messageId: "doNotUseInsecureUrl", line: 2}, { messageId: "doNotUseInsecureUrl", line: 3}, @@ -121,6 +142,10 @@ ruleTester.run(ruleId, rule, { var x1 = \`http://www.\${multipartExample}.com\`; var y1 = \`ftp://www.\${multipartExample}.com\`; `, + output: ` + var x1 = \`https://www.\${multipartExample}.com\`; + var y1 = \`ftp://www.\${multipartExample}.com\`; + `, errors: [ { messageId: "doNotUseInsecureUrl", line: 2}, { messageId: "doNotUseInsecureUrl", line: 3}, @@ -132,6 +157,10 @@ ruleTester.run(ruleId, rule, { function f(x : string = 'http://www.example.com') {} function f(y : string = 'ftp://www.example.com') {} `, + output: ` + function f(x : string = "https://www.example.com") {} + function f(y : string = 'ftp://www.example.com') {} + `, errors: [ { messageId: "doNotUseInsecureUrl", line: 2}, { messageId: "doNotUseInsecureUrl", line: 3}, @@ -146,6 +175,12 @@ ruleTester.run(ruleId, rule, { var b1 = 'FtP://www.ban-file-example.com' var c1 = 'LDAp://www.ban-ldap-example.com' `, + output: ` + var a1 = "https://www.ban-example.com" + var a2 = "https://www.ban-example.com/path" + var b1 = 'FtP://www.ban-file-example.com' + var c1 = 'LDAp://www.ban-ldap-example.com' + `, errors: [ { messageId: "doNotUseInsecureUrl", line: 2}, { messageId: "doNotUseInsecureUrl", line: 3}, @@ -166,11 +201,47 @@ ruleTester.run(ruleId, rule, { ); }; `, + output: ` + const someSvg: React.FC = () => { + return ( + + + ); + }; + `, errors: [ { messageId: "doNotUseInsecureUrl", line: 4}, ], parser: testUtils.tsParser, parserOptions: testUtils.tsReactParserOptions, }, + { + // should escape the url string correctly + code: `var a1 = "http://moz\ti\tlla.org";`, + output: `var a1 = "https://moz\\ti\\tlla.org";`, + errors: [ + { messageId: "doNotUseInsecureUrl", line: 1}, + ], + }, + { + // should fix url in `` correctly + code: "var x1 = `http://foo${multipartExample} http://${multipartExample}.com`;", + output: "var x1 = `https://foo${multipartExample} http://${multipartExample}.com`;", + errors: [ + { messageId: "doNotUseInsecureUrl", line: 1}, + ], + + parserOptions: testUtils.moduleParserOptions + }, + { + // should escape the string and fix it properly in `` + code: `var a1 = \`http://moz\ti\tlla.org\`;`, + output: `var a1 = \`https://moz\\ti\\tlla.org\`;`, + errors: [ + { messageId: "doNotUseInsecureUrl", line: 1}, + ], + + parserOptions: testUtils.moduleParserOptions + }, ] }); \ No newline at end of file