Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: add chromium sandbox skipping #209

Merged
merged 6 commits into from Sep 23, 2020
Merged

feat: add chromium sandbox skipping #209

merged 6 commits into from Sep 23, 2020

Conversation

andrewmackrodt
Copy link
Contributor

@andrewmackrodt andrewmackrodt commented Sep 22, 2020

This PR adds an optional environment variable TRUSTED which can be used to skip puppeteer/chromium's sandbox usage, it defaults to false. Relates to #114 but our approach is slightly different, i.e. this change does not force the insecure setting unless set by the user.

Edit: linking to sample Docker usage using an image pushed to the DockerHub registry: https://github.com/andrewmackrodt/dockerfiles/tree/master/nvidia-snatcher#docker

fuckingrobot
fuckingrobot previously approved these changes Sep 22, 2020
jef
jef previously approved these changes Sep 23, 2020
@jef
Copy link
Owner

jef commented Sep 23, 2020

I really like this! Although, I think TRUSTED could be used in a lot of different contexts. Do you mind if we prefix this environment variable with BROWSER_ or something similar?

Thanks!

@andrewmackrodt
Copy link
Contributor Author

Do you mind if we prefix this environment variable with BROWSER_ or something similar?

Sure, I've added the BROWSER_ prefix. The variable on the config object has stayed as isTrusted as I feel Config.browser.isTrusted is verbose enough.

@jef
Copy link
Owner

jef commented Sep 23, 2020

Config.browser.isTrusted is verbose enough.

Agreed! Sorry, I wasn't clear there, but it all looks good from here!

Thank you!!

src/config.ts Outdated Show resolved Hide resolved
@andrewmackrodt andrewmackrodt mentioned this pull request Sep 23, 2020
Copy link
Owner

@jef jef left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great! Thanks!

I don't necessarily want to start managing a Dockerfile quite yet because it'll open another can of worms, so this is a great start for those interested.

Thanks!

@jef jef changed the title Add environment override "TRUSTED" to skip chromium sandbox feat: add chromium sandbox skipping Sep 23, 2020
@jef jef merged commit 2065680 into jef:main Sep 23, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants