Skip to content

Commit

Permalink
Merge pull request #39 from mozfreddyb/main
Browse files Browse the repository at this point in the history 
Add adjusted test and new auto fixer for 'no-insecure-url'
  • Loading branch information
@Vflouirac
Vflouirac committed Jun 14, 2023
2 parents b2b76f5 + b4a5be3 commit 31d6d68
Show file tree
Hide file tree
Showing 3 changed files with 133 additions and 18 deletions.
12 changes: 9 additions & 3 deletions docs/rules/no-insecure-url.md
View file
Expand Up @@ -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.

Expand All @@ -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?.*"]
}]
```

Expand All @@ -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://`...
Expand Down
68 changes: 53 additions & 15 deletions lib/rules/no-insecure-url.js
View file
Expand Up @@ -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",
Expand All @@ -45,6 +47,12 @@ module.exports = {
type: "string"
}
},
varExceptions: {
type: "array",
items: {
type: "string"
}
},
},
additionalProperties: false
}
Expand All @@ -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") {
Expand All @@ -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));
}
},
});
}
}
},
Expand All @@ -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);
}
}
});
}
}
},
};
},
};
71 changes: 71 additions & 0 deletions tests/lib/rules/no-insecure-url.js
View file
Expand Up @@ -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: `
Expand Down Expand Up @@ -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},
Expand All @@ -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},
Expand All @@ -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},
Expand All @@ -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},
Expand All @@ -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},
Expand All @@ -166,11 +201,47 @@ ruleTester.run(ruleId, rule, {
);
};
`,
output: `
const someSvg: React.FC = () => {
return (
<svg someOtherAttribute="https://ban-example.com/">
</svg>
);
};
`,
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
},
]
});

0 comments on commit 31d6d68

Please sign in to comment.