From dee331519cb7bd62928332af3194bbcb6c25987f Mon Sep 17 00:00:00 2001 From: Jerry Wu Date: Tue, 2 Jul 2019 03:36:50 -0700 Subject: [PATCH] fix: disable nodeIntegration & insecure resource warnings for localhost (#18814) * fix: disable remote host nodeIntegration warning for localhost In warnAboutNodeWithRemoteContent(), add a check to see if the hostname is "localhost" and prevent the warning message if it is. * fix: disable loading insecure resources warning for localhost In warnAboutInsecureResources(), filter out resources from localhost since they are most likely not a threat. * test: add tests for ignoring security warnings when using localhost Add tests for ignoring warning messages for the following scenarios: 1. node integration with remote content from localhost 2. loading insecure resources from localhost * test: fix insecure resource test * test: pass nodeIntegration with remote test on did-finish-load * test: maybe fix node integration test (error w/ conv circular struct) * test: update test description * test: use "load" event to check when nodeIntegration test has finished Instead of relying on the "did-finish-load" event, which may result in a race condition, add an "onload" handler that logs "loaded" to the console. This will execute _after_ the nodeIntegration check, so it can be safely used as a signal to indicate that the test is done. * test: rename base-page-security-load-message.html * fix: ignore enabled remote module warning for localhost * refactor: add isLocalhost() --- lib/renderer/security-warnings.ts | 18 ++++++- .../base-page-security-onload-message.html | 10 ++++ spec/security-warnings-spec.js | 48 +++++++++++++++++++ 3 files changed, 74 insertions(+), 2 deletions(-) create mode 100644 spec/fixtures/pages/base-page-security-onload-message.html diff --git a/lib/renderer/security-warnings.ts b/lib/renderer/security-warnings.ts index 48db42c7fffc..4e0ef4afd83f 100644 --- a/lib/renderer/security-warnings.ts +++ b/lib/renderer/security-warnings.ts @@ -58,6 +58,19 @@ const getIsRemoteProtocol = function () { } } +/** + * Checks if the current window is from localhost. + * + * @returns {boolean} - Is current window from localhost? + */ +const isLocalhost = function () { + if (!window || !window.location) { + return false + } + + return window.location.hostname === 'localhost' +} + /** * Tries to determine whether a CSP without `unsafe-eval` is set. * @@ -92,6 +105,7 @@ const warnAboutInsecureResources = function () { const resources = window.performance .getEntriesByType('resource') .filter(({ name }) => /^(http|ftp):/gi.test(name || '')) + .filter(({ name }) => new URL(name).hostname !== 'localhost') .map(({ name }) => `- ${name}`) .join('\n') @@ -115,7 +129,7 @@ const warnAboutInsecureResources = function () { * Logs a warning message about Node integration. */ const warnAboutNodeWithRemoteContent = function (nodeIntegration: boolean) { - if (!nodeIntegration) return + if (!nodeIntegration || isLocalhost()) return if (getIsRemoteProtocol()) { const warning = `This renderer process has Node.js integration enabled @@ -254,7 +268,7 @@ const warnAboutAllowedPopups = function () { // Logs a warning message about the remote module const warnAboutRemoteModuleWithRemoteContent = function (webPreferences?: Electron.WebPreferences) { - if (!webPreferences || !webPreferences.enableRemoteModule) return + if (!webPreferences || !webPreferences.enableRemoteModule || isLocalhost()) return if (getIsRemoteProtocol()) { const warning = `This renderer process has "enableRemoteModule" enabled diff --git a/spec/fixtures/pages/base-page-security-onload-message.html b/spec/fixtures/pages/base-page-security-onload-message.html new file mode 100644 index 000000000000..befc2752cab8 --- /dev/null +++ b/spec/fixtures/pages/base-page-security-onload-message.html @@ -0,0 +1,10 @@ + + + + + + + diff --git a/spec/security-warnings-spec.js b/spec/security-warnings-spec.js index c0fd0fe54f45..45141b4dd966 100644 --- a/spec/security-warnings-spec.js +++ b/spec/security-warnings-spec.js @@ -78,6 +78,24 @@ describe('security warnings', () => { w.loadURL(`http://127.0.0.1:8881/base-page-security.html`) }) + it('should not warn about Node.js integration with remote content from localhost', (done) => { + w = new BrowserWindow({ + show: false, + webPreferences: { + nodeIntegration: true + } + }) + w.webContents.once('console-message', (e, level, message) => { + expect(message).to.not.include('Node.js Integration with Remote Content') + + if (message === 'loaded') { + done() + } + }) + + w.loadURL(`http://localhost:8881/base-page-security-onload-message.html`) + }) + const generateSpecs = (description, webPreferences) => { describe(description, () => { it('should warn about disabled webSecurity', (done) => { @@ -189,6 +207,20 @@ describe('security warnings', () => { w.webContents.openDevTools() }) + it('should not warn about loading insecure-resources.html from localhost', (done) => { + w = new BrowserWindow({ + show: false, + webPreferences + }) + w.webContents.once('console-message', (e, level, message) => { + expect(message).to.not.include('insecure-resources.html') + done() + }) + + w.loadURL(`http://localhost:8881/insecure-resources.html`) + w.webContents.openDevTools() + }) + it('should warn about enabled remote module with remote content', (done) => { w = new BrowserWindow({ show: false, @@ -201,6 +233,22 @@ describe('security warnings', () => { w.loadURL(`http://127.0.0.1:8881/base-page-security.html`) }) + + it('should not warn about enabled remote module with remote content from localhost', (done) => { + w = new BrowserWindow({ + show: false, + webPreferences + }) + w.webContents.once('console-message', (e, level, message) => { + expect(message).to.not.include('enableRemoteModule') + + if (message === 'loaded') { + done() + } + }) + + w.loadURL(`http://localhost:8881/base-page-security-onload-message.html`) + }) }) }