diff --git a/.sonarrc b/.sonarrc index 3015ff7999a..c51a1388c55 100644 --- a/.sonarrc +++ b/.sonarrc @@ -13,6 +13,7 @@ "manifest-file-extension": "warning", "manifest-is-valid": "warning", "no-double-slash": "warning", - "no-friendly-error-pages": "warning" + "no-friendly-error-pages": "warning", + "no-html-only-headers": "warning" } } diff --git a/src/lib/rules/disallowed-headers/disallowed-headers.md b/src/lib/rules/disallowed-headers/disallowed-headers.md index 9bbdb0cb70f..816dbb21c06 100644 --- a/src/lib/rules/disallowed-headers/disallowed-headers.md +++ b/src/lib/rules/disallowed-headers/disallowed-headers.md @@ -64,7 +64,7 @@ Yes, you can use: should be ignored E.g. The following configuration will make the rule allow responses -to be served with the `Server` HTTP headers, but not with `Custom-Header`. +to be served with the `Server` HTTP header, but not with `Custom-Header`. ```json "disallowed-headers": [ "warning", { diff --git a/src/lib/rules/disallowed-headers/disallowed-headers.ts b/src/lib/rules/disallowed-headers/disallowed-headers.ts index 6ebfa609862..1570228bba5 100644 --- a/src/lib/rules/disallowed-headers/disallowed-headers.ts +++ b/src/lib/rules/disallowed-headers/disallowed-headers.ts @@ -8,6 +8,7 @@ import { IFetchEndEvent, IRule, IRuleBuilder } from '../../interfaces'; // eslint-disable-line no-unused-vars import { RuleContext } from '../../rule-context'; // eslint-disable-line no-unused-vars +import { getIncludedHeaders, mergeIgnoreIncludeArrays } from '../../util/rule-helpers'; // ------------------------------------------------------------------------------ // Public @@ -25,55 +26,23 @@ const rule: IRuleBuilder = { 'x-version' ]; - const init = () => { + const loadRuleConfigs = () => { + const includeHeaders = (context.ruleOptions && context.ruleOptions.include) || []; + const ignoreHeaders = (context.ruleOptions && context.ruleOptions.ignore) || []; - let includeHeaders = (context.ruleOptions && context.ruleOptions.include) || []; - let ignoreHeaders = (context.ruleOptions && context.ruleOptions.ignore) || []; - - includeHeaders = includeHeaders.map((e) => { - return e.toLowerCase(); - }); - - ignoreHeaders = ignoreHeaders.map((e) => { - return e.toLowerCase(); - }); - - // Add headers specified under 'include'. - includeHeaders.forEach((e) => { - if (!disallowedHeaders.includes(e)) { - disallowedHeaders.push(e); - } - }); - - // Remove headers specified under 'ignore'. - disallowedHeaders = disallowedHeaders.filter((e) => { - return !ignoreHeaders.includes(e); - }); - - }; - - const findDisallowedHeaders = (headers: object) => { - const headersFound = []; - - for (const [key] of Object.entries(headers)) { - if (disallowedHeaders.includes(key.toLowerCase())) { - headersFound.push(key); - } - } - - return headersFound; + disallowedHeaders = mergeIgnoreIncludeArrays(disallowedHeaders, ignoreHeaders, includeHeaders); }; const validate = (fetchEnd: IFetchEndEvent) => { const { element, resource } = fetchEnd; - const headers = findDisallowedHeaders(fetchEnd.response.headers); + const headers = getIncludedHeaders(fetchEnd.response.headers, disallowedHeaders); if (headers.length > 0) { context.report(resource, element, `Disallowed HTTP header${headers.length > 1 ? 's' : ''} found: ${headers.join(', ')}`); } }; - init(); + loadRuleConfigs(); return { 'fetch::end': validate, diff --git a/src/lib/rules/no-html-only-headers/no-html-only-headers.md b/src/lib/rules/no-html-only-headers/no-html-only-headers.md new file mode 100644 index 00000000000..74a33d0c835 --- /dev/null +++ b/src/lib/rules/no-html-only-headers/no-html-only-headers.md @@ -0,0 +1,110 @@ +# Disallow unneeded HTTP headers for non-HTML resources (`no-html-only-headers`) + +`no-html-only-headers` warns against responding with HTTP headers that +are not needed for non-HTML resources. + + +## Why is this important? + +Some HTTP headers do not make sense to be send for non-HTML +resources, as sending them does not provide any value to users, +and just contributes to header bloat. + + +## What does the rule check? + +The rule checks if non-HTML responses include any of the following +HTTP headers: + +* `Content-Security-Policy` +* `X-Content-Security-Policy` +* `X-Frame-Options` +* `X-UA-Compatible` +* `X-WebKit-CSP` +* `X-XSS-Protection` + +Examples that **trigger** the rule: + +Response for `/test.js`: + +```text + +HTTP/1.1 200 OK + +Content-Type: application/javascript +... +Content-Security-Policy: default-src 'none' +Content-Type: application/javascript; charset=utf-8 +X-Content-Security-Policy: default-src 'none' +X-Frame-Options: DENY +X-UA-Compatible: IE=Edge, +X-WebKit-CSP: default-src 'none' +X-XSS-Protection: 1; mode=block +... +``` + +Response for `/test.html`: + +```text +HTTP/1.1 200 OK + +Content-Type: x/y +... +Content-Security-Policy: default-src 'none' +Content-Type: application/javascript; charset=utf-8 +X-Content-Security-Policy: default-src 'none' +X-Frame-Options: DENY +X-UA-Compatible: IE=Edge, +X-WebKit-CSP: default-src 'none' +X-XSS-Protection: 1; mode=block +... +``` + +Examples that **pass** the rule: + +Response for `/test.js`: + +```text +HTTP/1.1 200 OK + +Content-Type: application/javascript +... +``` + +Response for `/test.html`: + +```text +HTTP/1.1 200 OK + +Content-Type: text/html +... +Content-Security-Policy: default-src 'none' +Content-Type: application/javascript; charset=utf-8 +X-Content-Security-Policy: default-src 'none' +X-Frame-Options: DENY +X-UA-Compatible: IE=Edge, +X-WebKit-CSP: default-src 'none' +X-XSS-Protection: 1; mode=block +... +``` + + +## Can the rule be configured? + +Yes, you can use: + + * `include` to specify additional HTTP headers that should + be disallowed for non-HTML resources + * `ignore` to specify which of the disallowed HTTP headers + should be ignored + +E.g. The following configuration will make the rule allow non-HTML +resources to be served with the `Content-Security-Policy` HTTP header, +but not with `Custom-Header`. + +```json +"no-html-only-headers": [ "warning", { + "ignore": ["Content-Security-Policy"], + "include": ["Custom-Header"] +}] +``` diff --git a/src/lib/rules/no-html-only-headers/no-html-only-headers.ts b/src/lib/rules/no-html-only-headers/no-html-only-headers.ts new file mode 100644 index 00000000000..cb737e11f3b --- /dev/null +++ b/src/lib/rules/no-html-only-headers/no-html-only-headers.ts @@ -0,0 +1,113 @@ +/** + * @fileoverview Check if non HTML resources responses contain certain + * unneeded HTTP headers. + */ + +// ------------------------------------------------------------------------------ +// Requirements +// ------------------------------------------------------------------------------ + +import { IFetchEndEvent, IResponse, IRule, IRuleBuilder } from '../../interfaces'; // eslint-disable-line no-unused-vars +import { RuleContext } from '../../rule-context'; // eslint-disable-line no-unused-vars +import { getIncludedHeaders, mergeIgnoreIncludeArrays } from '../../util/rule-helpers'; + +// ------------------------------------------------------------------------------ +// Public +// ------------------------------------------------------------------------------ + +const rule: IRuleBuilder = { + create(context: RuleContext): IRule { + + let unneededHeaders = [ + 'content-security-policy', + 'x-content-security-policy', + 'x-frame-options', + 'x-ua-compatible', + 'x-webkit-csp', + 'x-xss-protection' + ]; + + const loadRuleConfigs = () => { + const includeHeaders = (context.ruleOptions && context.ruleOptions.include) || []; + const ignoreHeaders = (context.ruleOptions && context.ruleOptions.ignore) || []; + + unneededHeaders = mergeIgnoreIncludeArrays(unneededHeaders, ignoreHeaders, includeHeaders); + }; + + const willBeTreatedAsHTML = (response: IResponse) => { + const mediaType = response.headers['content-type'].split(';')[0].trim(); + + // By default, browsers will treat resource sent with the + // following media types as HTML documents. + + if (['text/html', 'application/xhtml+xml'].includes(mediaType)) { + return true; + } + + // That is not the situation for other cases where the media + // type is in the form of `/`. + + if (mediaType.indexOf('/') > 0) { + return false; + } + + // If the media type is not specified or invalid, browser + // will try to sniff the content. + // + // https://mimesniff.spec.whatwg.org/ + // + // At this point, even if browsers may decide to treat + // the content as a HTML document, things are obviously + // not done correctly, so the decision was to not try to + // also sniff the content, and instead, just signal this + // as a problem. + + return false; + }; + + const checkHeaders = (fetchEnd: IFetchEndEvent) => { + const { element, resource, response } = fetchEnd; + + if (!willBeTreatedAsHTML(response)) { + const headers = getIncludedHeaders(response.headers, unneededHeaders); + + if (headers.length > 0) { + context.report(resource, element, `Unneeded HTTP header${headers.length > 1 ? 's' : ''} found: ${headers.join(', ')}`); + } + } + }; + + loadRuleConfigs(); + + return { + 'fetch::end': checkHeaders, + 'targetfetch::end': checkHeaders + }; + }, + meta: { + docs: { + category: 'performance', + description: 'Disallow unneeded HTTP headers for non-HTML resources', + recommended: true + }, + fixable: 'code', + schema: { + additionalProperties: false, + definitions: { + 'string-array': { + items: { type: 'string' }, + minItems: 1, + type: 'array', + uniqueItems: true + } + }, + properties: { + ignore: { $ref: '#/definitions/string-array' }, + include: { $ref: '#/definitions/string-array' } + }, + type: ['object', null] + } + } +}; + +module.exports = rule; diff --git a/src/lib/util/rule-helpers.ts b/src/lib/util/rule-helpers.ts index eb104f57b33..d00fa68faa3 100644 --- a/src/lib/util/rule-helpers.ts +++ b/src/lib/util/rule-helpers.ts @@ -1,10 +1,52 @@ import * as path from 'path'; import * as d from 'debug'; +export const getIncludedHeaders = (headers: object = {}, headerList: Array = []) => { + const result = []; + + for (const [key] of Object.entries(headers)) { + if (headerList.includes(key.toLowerCase())) { + result.push(key); + } + } + + return result; +}; + export const getRuleName = (dirname: string) => { return path.basename(dirname); }; +export const mergeIgnoreIncludeArrays = (originalArray: Array = [], ignoreArray: Array = [], includeArray: Array = []) => { + + let result = originalArray.map((e) => { + return e.toLowerCase(); + }); + + const include = includeArray.map((e) => { + return e.toLowerCase(); + }); + + const ignore = ignoreArray.map((e) => { + return e.toLowerCase(); + }); + + // Add elements specified under 'include'. + include.forEach((e) => { + if (!result.includes(e)) { + result.push(e); + } + }); + + // Remove elements specified under 'ignore'. + result = result.filter((e) => { + return !ignore.includes(e); + }); + + return result; + +}; + export const ruleDebug = (dirname: string) => { return d(`sonar:rules:${getRuleName(dirname)}`); }; diff --git a/tests/lib/rules/disallowed-headers/tests.ts b/tests/lib/rules/disallowed-headers/tests.ts index 15d27e31cfd..ed6d4ef4c6d 100644 --- a/tests/lib/rules/disallowed-headers/tests.ts +++ b/tests/lib/rules/disallowed-headers/tests.ts @@ -82,7 +82,7 @@ const testsForIgnoreConfigs: Array = [ content: '', headers: { Server: 'test', - 'X-Test1': 'test' + 'X-Test-1': 'test' } } } @@ -93,23 +93,23 @@ const testsForIncludeConfigs: Array = [ { name: `Response contains headers that are disallowed because of the configurations`, reports: [ - { message: `Disallowed HTTP headers found: server, x-test1, x-test2` }, - { message: `Disallowed HTTP headers found: server, x-test2` } + { message: `Disallowed HTTP headers found: server, x-test-1, x-test-2` }, + { message: `Disallowed HTTP headers found: server, x-test-2` } ], serverConfig: { '/': { content: htmlPage, headers: { Server: 'test', - 'X-Test1': 'test', - 'X-Test2': 'test' + 'X-Test-1': 'test', + 'X-Test-2': 'test' } }, '/test.js': { content: '', headers: { Server: 'test', - 'X-Test2': 'test' + 'X-Test-2': 'test' } } } @@ -120,7 +120,7 @@ const testsForConfigs: Array = [ { name: `Response contains headers that are both disallowed and ignored because of configurations`, reports: [ - { message: `Disallowed HTTP headers found: x-powered-by, x-test1` }, + { message: `Disallowed HTTP headers found: x-powered-by, x-test-1` }, { message: `Disallowed HTTP header found: x-powered-by` } ], serverConfig: { @@ -129,8 +129,8 @@ const testsForConfigs: Array = [ headers: { Server: 'test', 'X-Powered-By': 'test', - 'X-Test1': 'test', - 'X-Test2': 'test' + 'X-Test-1': 'test', + 'X-Test-2': 'test' } }, '/test.js': { @@ -138,8 +138,8 @@ const testsForConfigs: Array = [ headers: { Server: 'test', 'X-Powered-By': 'test', - 'X-Test2': 'test', - 'X-Test3': 'test' + 'X-Test-2': 'test', + 'X-Test-3': 'test' } } } @@ -147,9 +147,9 @@ const testsForConfigs: Array = [ ]; ruleRunner.testRule(ruleName, testsForDefaults); -ruleRunner.testRule(ruleName, testsForIgnoreConfigs, { ignore: ['Server', 'X-Powered-By', 'X-Test1'] }); -ruleRunner.testRule(ruleName, testsForIncludeConfigs, { include: ['Server', 'X-Test1', 'X-Test2'] }); +ruleRunner.testRule(ruleName, testsForIgnoreConfigs, { ignore: ['Server', 'X-Powered-By', 'X-Test-1'] }); +ruleRunner.testRule(ruleName, testsForIncludeConfigs, { include: ['Server', 'X-Test-1', 'X-Test-2'] }); ruleRunner.testRule(ruleName, testsForConfigs, { - ignore: ['Server', 'X-Test2', 'X-Test3'], - include: ['X-Powered-By', 'X-Test1', 'X-Test2'] + ignore: ['Server', 'X-Test-2', 'X-Test-3'], + include: ['X-Powered-By', 'X-Test-1', 'X-Test-2'] }); diff --git a/tests/lib/rules/no-html-only-headers/tests.ts b/tests/lib/rules/no-html-only-headers/tests.ts new file mode 100644 index 00000000000..7967b592fcc --- /dev/null +++ b/tests/lib/rules/no-html-only-headers/tests.ts @@ -0,0 +1,182 @@ +/* eslint sort-keys: 0, no-undefined: 0 */ + +import { RuleTest } from '../../../helpers/rule-test-type'; // eslint-disable-line no-unused-vars +import * as ruleRunner from '../../../helpers/rule-runner'; +import { getRuleName } from '../../../../src/lib/util/rule-helpers'; + +const ruleName = getRuleName(__dirname); + +const htmlPage = +` + + + test + + + + +`; + +const testsForDefaults: Array = [ + { + name: `Non HTML resource is not served with unneded headers`, + serverConfig: { + '/': { + content: htmlPage, + headers: { + 'Content-Security-Policy': `default-src 'none'`, + 'Content-Type': 'text/html; charset=utf-8' + } + }, + '/test.js': { headers: { 'Content-Type': 'application/javascript; charset=utf-8' } } + } + }, + { + name: `Non HTML resource is served with one unneded headers`, + reports: [{ message: `Unneeded HTTP header found: content-security-policy` }], + serverConfig: { + '/': { + content: htmlPage, + headers: { + 'Content-Security-Policy': `default-src 'none'`, + 'Content-Type': 'application/xhtml+xml; charset=utf-8' + } + }, + '/test.js': { + headers: { + 'Content-Security-Policy': 'default-src "none"', + 'Content-Type': 'application/javascript; charset=utf-8' + } + } + } + }, + { + name: `Non HTML resource is served with multiple unneded headers`, + reports: [{ message: `Unneeded HTTP headers found: content-security-policy, x-content-security-policy, x-frame-options, x-ua-compatible, x-webkit-csp, x-xss-protection`}], + serverConfig: { + '/': { + content: htmlPage, + headers: { + 'Content-Security-Policy': `default-src 'none'`, + 'Content-Type': 'text/html; charset=utf-8', + 'X-Content-Security-Policy': `default-src 'none'`, + 'X-Frame-Options': 'DENY', + 'X-UA-Compatible': 'IE=Edge', + 'X-WebKit-CSP': `default-src 'none'`, + 'X-XSS-Protection': '1; mode=block' + } + }, + '/test.js': { + headers: { + 'Content-Security-Policy': `default-src 'none'`, + 'Content-Type': 'application/javascript; charset=utf-8', + 'X-Content-Security-Policy': `default-src 'none'`, + 'X-Frame-Options': 'DENY', + 'X-UA-Compatible': 'IE=Edge', + 'X-WebKit-CSP': `default-src 'none'`, + 'X-XSS-Protection': '1; mode=block' + } + } + } + }, + { + name: `HTML document with valid but incorrect media type is treated as non-HTML resource`, + reports: [{ message: `Unneeded HTTP headers found: content-security-policy, x-ua-compatible` }], + serverConfig: { + '/': { + headers: { + 'Content-Security-Policy': `default-src 'none'`, + 'Content-Type': 'x/x', + 'X-UA-Compatible': 'IE=Edge' + } + } + } + } + + // Note: There are no tests for invalid media types as Express + // (more specifically, content-type) doesn't allow them. +]; + +const testsForIgnoreConfigs: Array = [ + { + name: `Non HTML resource is served with one unneded headers but ignored because of the configurations`, + serverConfig: { + '/': { + content: htmlPage, + headers: { + 'Content-Security-Policy': `default-src 'none'`, + 'Content-Type': 'text/html; charset=utf-8', + 'X-UA-Compatible': 'IE=Edge' + } + }, + '/test.js': { + headers: { + 'Content-Security-Policy': 'default-src "none"', + 'Content-Type': 'application/javascript; charset=utf-8', + 'X-UA-Compatible': 'IE=Edge' + } + } + } + } +]; + +const testsForIncludeConfigs: Array = [ + { + name: `Non HTML resource is served with unneded headers also because of the configurations`, + reports: [{ message: `Unneeded HTTP headers found: content-security-policy, x-test-1, x-ua-compatible` }], + serverConfig: { + '/': { + content: htmlPage, + headers: { + 'Content-Security-Policy': `default-src 'none'`, + 'Content-Type': 'text/html; charset=utf-8', + 'X-Test-1': 'test', + 'X-Test-2': 'test' + } + }, + '/test.js': { + headers: { + 'Content-Security-Policy': 'default-src "none"', + 'Content-Type': 'application/javascript; charset=utf-8', + 'X-Test-1': 'test', + 'X-UA-Compatible': 'IE=Edge' + } + } + } + } +]; + +const testsForConfigs: Array = [ + { + name: `Non HTML resource is served with unneded headers that are both ignore and because of the configurations`, + reports: [{ message: `Unneeded HTTP headers found: x-test-1, x-ua-compatible` }], + serverConfig: { + '/': { + content: htmlPage, + headers: { + 'Content-Security-Policy': `default-src 'none'`, + 'Content-Type': 'text/html; charset=utf-8', + 'X-Test-1': 'test', + 'X-Test-2': 'test' + } + }, + '/test.js': { + headers: { + 'Content-Security-Policy': 'default-src "none"', + 'Content-Type': 'application/javascript; charset=utf-8', + 'X-Test-1': 'test', + 'X-Test-2': 'test', + 'X-UA-Compatible': 'IE=Edge' + } + } + } + } +]; + +ruleRunner.testRule(ruleName, testsForDefaults); +ruleRunner.testRule(ruleName, testsForIgnoreConfigs, { ignore: ['Content-Security-Policy', 'X-UA-Compatible', 'X-Test-1'] }); +ruleRunner.testRule(ruleName, testsForIncludeConfigs, { include: ['Content-Security-Policy', 'X-Test-1', 'X-Test-2'] }); +ruleRunner.testRule(ruleName, testsForConfigs, { + ignore: ['Content-Security-Policy', 'X-Test-2', 'X-Test-3'], + include: ['X-Test-1', 'X-Test-2', 'X-UA-Compatible'] +});