From eb7bd9d7d3a7800e9ed3d77a19aaf4336587026c Mon Sep 17 00:00:00 2001 From: Andrey Lushnikov Date: Mon, 12 Nov 2018 23:26:16 -0800 Subject: [PATCH] test: setup sandbox on linux (#3530) Drop all the `--no-sandbox` bits from tests and infrastructure. Instead, configure Travis to enable user namespace clone. --- .ci/node6/Dockerfile.linux | 8 ++++++++ .ci/node8/Dockerfile.linux | 8 ++++++++ .travis.yml | 1 + test/headful.spec.js | 1 - test/puppeteer.spec.js | 6 +++--- test/test.js | 1 - utils/browser/test.js | 8 ++------ utils/doclint/check_public_api/test/test.js | 2 +- utils/doclint/cli.js | 2 +- utils/protocol-types-generator/index.js | 1 - 10 files changed, 24 insertions(+), 14 deletions(-) diff --git a/.ci/node6/Dockerfile.linux b/.ci/node6/Dockerfile.linux index a89d32e4..77b41673 100644 --- a/.ci/node6/Dockerfile.linux +++ b/.ci/node6/Dockerfile.linux @@ -7,3 +7,11 @@ RUN apt-get update && \ libxcomposite1 libxcursor1 libxdamage1 libxext6 libxfixes3 libxi6 libxrandr2 libxrender1 libxss1 \ libxtst6 ca-certificates fonts-liberation libappindicator1 libnss3 lsb-release xdg-utils wget && \ rm -rf /var/lib/apt/lists/* + +# Add user so we don't need --no-sandbox. +RUN groupadd -r pptruser && useradd -r -g pptruser -G audio,video pptruser \ + && mkdir -p /home/pptruser/Downloads \ + && chown -R pptruser:pptruser /home/pptruser + +# Run everything after as non-privileged user. +USER pptruser diff --git a/.ci/node8/Dockerfile.linux b/.ci/node8/Dockerfile.linux index a0066914..26f11979 100644 --- a/.ci/node8/Dockerfile.linux +++ b/.ci/node8/Dockerfile.linux @@ -7,3 +7,11 @@ RUN apt-get update && \ libxcomposite1 libxcursor1 libxdamage1 libxext6 libxfixes3 libxi6 libxrandr2 libxrender1 libxss1 \ libxtst6 ca-certificates fonts-liberation libappindicator1 libnss3 lsb-release xdg-utils wget && \ rm -rf /var/lib/apt/lists/* + +# Add user so we don't need --no-sandbox. +RUN groupadd -r pptruser && useradd -r -g pptruser -G audio,video pptruser \ + && mkdir -p /home/pptruser/Downloads \ + && chown -R pptruser:pptruser /home/pptruser + +# Run everything after as non-privileged user. +USER pptruser diff --git a/.travis.yml b/.travis.yml index b6249203..e631c078 100644 --- a/.travis.yml +++ b/.travis.yml @@ -12,6 +12,7 @@ cache: - node_modules # allow headful tests before_install: + - "sysctl kernel.unprivileged_userns_clone=1" - "export DISPLAY=:99.0" - "sh -e /etc/init.d/xvfb start" script: diff --git a/test/headful.spec.js b/test/headful.spec.js index 75b6feeb..cefc1d76 100644 --- a/test/headful.spec.js +++ b/test/headful.spec.js @@ -55,7 +55,6 @@ module.exports.addTests = function({testRunner, expect, defaultBrowserOptions}) const extensionOptions = Object.assign({}, defaultBrowserOptions, { headless: false, args: [ - '--no-sandbox', `--disable-extensions-except=${extensionPath}`, `--load-extension=${extensionPath}`, ], diff --git a/test/puppeteer.spec.js b/test/puppeteer.spec.js index 015d37ae..507b8687 100644 --- a/test/puppeteer.spec.js +++ b/test/puppeteer.spec.js @@ -116,7 +116,7 @@ module.exports.addTests = function({testRunner, expect, defaultBrowserOptions}) it('userDataDir argument', async({server}) => { const userDataDir = await mkdtempAsync(TMP_FOLDER); const options = Object.assign({}, defaultBrowserOptions); - options.args = [`--user-data-dir=${userDataDir}`].concat(options.args); + options.args = [`--user-data-dir=${userDataDir}`].concat(options.args || []); const browser = await puppeteer.launch(options); expect(fs.readdirSync(userDataDir).length).toBeGreaterThan(0); await browser.close(); @@ -209,7 +209,7 @@ module.exports.addTests = function({testRunner, expect, defaultBrowserOptions}) }); it('should support the pipe argument', async() => { const options = Object.assign({}, defaultBrowserOptions); - options.args = ['--remote-debugging-pipe'].concat(options.args); + options.args = ['--remote-debugging-pipe'].concat(options.args || []); const browser = await puppeteer.launch(options); expect(browser.wsEndpoint()).toBe(''); const page = await browser.newPage(); @@ -256,7 +256,7 @@ module.exports.addTests = function({testRunner, expect, defaultBrowserOptions}) it('should have custom url when launching browser', async function({server}) { const customUrl = server.PREFIX + '/empty.html'; const options = Object.assign({}, defaultBrowserOptions); - options.args = [customUrl].concat(options.args); + options.args = [customUrl].concat(options.args || []); const browser = await puppeteer.launch(options); const pages = await browser.pages(); expect(pages.length).toBe(1); diff --git a/test/test.js b/test/test.js index 9330b6bd..8dbf0900 100644 --- a/test/test.js +++ b/test/test.js @@ -47,7 +47,6 @@ const defaultBrowserOptions = { slowMo, headless, dumpio: (process.env.DUMPIO || 'false').trim().toLowerCase() === 'true', - args: ['--no-sandbox'] }; let parallel = 1; diff --git a/utils/browser/test.js b/utils/browser/test.js index 86fb5389..2e238924 100644 --- a/utils/browser/test.js +++ b/utils/browser/test.js @@ -15,10 +15,6 @@ const {it, xit, fit} = testRunner; const {afterAll, beforeAll, afterEach, beforeEach} = testRunner; const {expect} = new Matchers(); -const defaultBrowserOptions = { - args: ['--no-sandbox'] -}; - beforeAll(async state => { const assetsPath = path.join(__dirname, '..', '..', 'test', 'assets'); const port = 8998; @@ -27,7 +23,7 @@ beforeAll(async state => { PREFIX: `http://localhost:${port}`, EMPTY_PAGE: `http://localhost:${port}/empty.html`, }; - state.browser = await puppeteer.launch(defaultBrowserOptions); + state.browser = await puppeteer.launch(); }); afterAll(async state => { @@ -54,7 +50,7 @@ afterEach(async state => { describe('Puppeteer-Web', () => { it('should work over web socket', async({page, serverConfig}) => { - const browser2 = await puppeteer.launch(defaultBrowserOptions); + const browser2 = await puppeteer.launch(); // Use in-page puppeteer to create a new page and navigate it to the EMPTY_PAGE await page.evaluate(async(browserWSEndpoint, serverConfig) => { const puppeteer = require('puppeteer'); diff --git a/utils/doclint/check_public_api/test/test.js b/utils/doclint/check_public_api/test/test.js index 61a49dde..503a9fab 100644 --- a/utils/doclint/check_public_api/test/test.js +++ b/utils/doclint/check_public_api/test/test.js @@ -34,7 +34,7 @@ let browser; let page; beforeAll(async function() { - browser = await puppeteer.launch({args: ['--no-sandbox']}); + browser = await puppeteer.launch(); page = await browser.newPage(); }); diff --git a/utils/doclint/cli.js b/utils/doclint/cli.js index 973af64c..57fe39a5 100755 --- a/utils/doclint/cli.js +++ b/utils/doclint/cli.js @@ -45,7 +45,7 @@ async function run() { messages.push(...await preprocessor.runCommands(mdSources, VERSION)); messages.push(...await preprocessor.ensureReleasedAPILinks([readme], VERSION)); - const browser = await puppeteer.launch({args: ['--no-sandbox']}); + const browser = await puppeteer.launch(); const page = await browser.newPage(); const checkPublicAPI = require('./check_public_api'); const jsSources = await Source.readdir(path.join(PROJECT_DIR, 'lib'), '.js'); diff --git a/utils/protocol-types-generator/index.js b/utils/protocol-types-generator/index.js index 33098b37..c1d46ce5 100644 --- a/utils/protocol-types-generator/index.js +++ b/utils/protocol-types-generator/index.js @@ -4,7 +4,6 @@ const puppeteer = require('../..'); module.exports = puppeteer.launch({ pipe: false, executablePath: process.env.CHROME, - args: ['--no-sandbox', '--disable-dev-shm-usage'] }).then(async browser => { const origin = browser.wsEndpoint().match(/ws:\/\/([0-9A-Za-z:\.]*)\//)[1]; const page = await browser.newPage();