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: UtilityProcess API #34980
feat: UtilityProcess API #34980
Conversation
array<EnvironmentVariable> environment; | ||
}; | ||
|
||
[ServiceSandbox=sandbox.mojom.Sandbox.kNoSandbox] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🤔 It could be interesting to allow users to sandbox their utility processes to various degrees!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agreed, I was also hoping to expose this but current content API is not flexible and extracts the value from mojom parameter. If we can add a small patch then we can make the sandbox property configurable https://source.chromium.org/chromium/chromium/src/+/main:content/public/browser/service_process_host.h;l=141-158.
Also need to think about which ones are supported via this API from the list https://source.chromium.org/chromium/chromium/src/+/main:sandbox/policy/mojom/sandbox.mojom, we would also need to document what syscalls are allowed/disallowed, paths accessible for each of the type to make this API understandable.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wonder if we could provide full control over the sandbox? Maybe not that useful... but interesting! e.g. allow the app to specify individual paths/libraries/syscalls etc to allow.
504a928
to
d2eee14
Compare
5f8e796
to
b669d3f
Compare
@deepak1556 it would be great to get an expanded explainer for this! Adding |
Sorry I was planning to update before requesting review, @nornagon updated the explainer section, PTAL. Thanks! |
9107e61
to
b189445
Compare
51a20b5
to
b9dc071
Compare
b9dc071
to
c57dfb5
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
API LGTM
Also, I don't want to miss @erickzhao's comment that we should add more documentation for this in our guides about process types. |
Thank you for taking the time to do a thorough review of this change!
Yup definitely, I will address it before merging the PR. |
Have created #36074 as follow-up for the docs change. Allows for easier reivew, I will merge this PR today once build is green. |
Failing test is unrelated and is also flaky on main branch, merging. |
Release Notes Persisted
|
I was unable to backport this PR to "21-x-y" cleanly; |
* chore: initial scaffolding * chore: implement interface and docs * chore: address code style review * fix: cleanup of utility process on shutdown * chore: simplify NodeBindings::CreateEnvironment * chore: rename disableLibraryValidation => allowLoadingUnsignedLibraries * chore: implement process.parentPort * chore(posix): implement stdio pipe interface * chore(win): implement stdio interface * chore: reenable SetNodeOptions for utility process * chore: add specs * chore: fix lint * fix: update kill API * fix: update process.parentPort API * fix: exit event * docs: update exit event * fix: tests on linux * chore: expand on some comments * fix: shutdown of pipe reader Avoid logging since it is always the case that reader end of pipe will terminate after the child process. * fix: remove exit code check for crash spec * fix: rm PR_SET_NO_NEW_PRIVS for unsandbox utility process * chore: fix incorrect rebase * fix: address review feedback * chore: rename utility_process -> utility * chore: update docs * chore: cleanup c++ implemantation * fix: leak in NodeServiceHost impl * chore: minor cleanup * chore: cleanup JS implementation * chore: flip default stdio to inherit * fix: some api improvements * Support cwd option * Remove path restriction for modulePath * Rewire impl for env support * fix: add tests for cwd and env option * chore: alt impl for reading stdio handles * chore: support message queuing * chore: fix lint * chore: new UtilityProcess => utilityProcess.fork * fix: support for uncaught exception exits * chore: remove process.execArgv as default * fix: windows build * fix: style changes * fix: docs and style changes * chore: update patches * spec: disable flaky test on win32 arm CI Co-authored-by: PatchUp <73610968+patchup[bot]@users.noreply.github.com> Co-authored-by: Robo <hop2deep@gmail.com>
I have automatically backported this PR to "22-x-y", please check out #36089 |
* chore: initial scaffolding * chore: implement interface and docs * chore: address code style review * fix: cleanup of utility process on shutdown * chore: simplify NodeBindings::CreateEnvironment * chore: rename disableLibraryValidation => allowLoadingUnsignedLibraries * chore: implement process.parentPort * chore(posix): implement stdio pipe interface * chore(win): implement stdio interface * chore: reenable SetNodeOptions for utility process * chore: add specs * chore: fix lint * fix: update kill API * fix: update process.parentPort API * fix: exit event * docs: update exit event * fix: tests on linux * chore: expand on some comments * fix: shutdown of pipe reader Avoid logging since it is always the case that reader end of pipe will terminate after the child process. * fix: remove exit code check for crash spec * fix: rm PR_SET_NO_NEW_PRIVS for unsandbox utility process * chore: fix incorrect rebase * fix: address review feedback * chore: rename utility_process -> utility * chore: update docs * chore: cleanup c++ implemantation * fix: leak in NodeServiceHost impl * chore: minor cleanup * chore: cleanup JS implementation * chore: flip default stdio to inherit * fix: some api improvements * Support cwd option * Remove path restriction for modulePath * Rewire impl for env support * fix: add tests for cwd and env option * chore: alt impl for reading stdio handles * chore: support message queuing * chore: fix lint * chore: new UtilityProcess => utilityProcess.fork * fix: support for uncaught exception exits * chore: remove process.execArgv as default * fix: windows build * fix: style changes * fix: docs and style changes * chore: update patches * spec: disable flaky test on win32 arm CI Co-authored-by: PatchUp <73610968+patchup[bot]@users.noreply.github.com> Co-authored-by: Robo <hop2deep@gmail.com>
* feat: UtilityProcess API (#34980) * chore: initial scaffolding * chore: implement interface and docs * chore: address code style review * fix: cleanup of utility process on shutdown * chore: simplify NodeBindings::CreateEnvironment * chore: rename disableLibraryValidation => allowLoadingUnsignedLibraries * chore: implement process.parentPort * chore(posix): implement stdio pipe interface * chore(win): implement stdio interface * chore: reenable SetNodeOptions for utility process * chore: add specs * chore: fix lint * fix: update kill API * fix: update process.parentPort API * fix: exit event * docs: update exit event * fix: tests on linux * chore: expand on some comments * fix: shutdown of pipe reader Avoid logging since it is always the case that reader end of pipe will terminate after the child process. * fix: remove exit code check for crash spec * fix: rm PR_SET_NO_NEW_PRIVS for unsandbox utility process * chore: fix incorrect rebase * fix: address review feedback * chore: rename utility_process -> utility * chore: update docs * chore: cleanup c++ implemantation * fix: leak in NodeServiceHost impl * chore: minor cleanup * chore: cleanup JS implementation * chore: flip default stdio to inherit * fix: some api improvements * Support cwd option * Remove path restriction for modulePath * Rewire impl for env support * fix: add tests for cwd and env option * chore: alt impl for reading stdio handles * chore: support message queuing * chore: fix lint * chore: new UtilityProcess => utilityProcess.fork * fix: support for uncaught exception exits * chore: remove process.execArgv as default * fix: windows build * fix: style changes * fix: docs and style changes * chore: update patches * spec: disable flaky test on win32 arm CI Co-authored-by: PatchUp <73610968+patchup[bot]@users.noreply.github.com> Co-authored-by: Robo <hop2deep@gmail.com> * chore: update patches * docs: add utility process info to tutorial docs (#36074) Co-authored-by: trop[bot] <37223003+trop[bot]@users.noreply.github.com> Co-authored-by: Robo <hop2deep@gmail.com>
* chore: initial scaffolding * chore: implement interface and docs * chore: address code style review * fix: cleanup of utility process on shutdown * chore: simplify NodeBindings::CreateEnvironment * chore: rename disableLibraryValidation => allowLoadingUnsignedLibraries * chore: implement process.parentPort * chore(posix): implement stdio pipe interface * chore(win): implement stdio interface * chore: reenable SetNodeOptions for utility process * chore: add specs * chore: fix lint * fix: update kill API * fix: update process.parentPort API * fix: exit event * docs: update exit event * fix: tests on linux * chore: expand on some comments * fix: shutdown of pipe reader Avoid logging since it is always the case that reader end of pipe will terminate after the child process. * fix: remove exit code check for crash spec * fix: rm PR_SET_NO_NEW_PRIVS for unsandbox utility process * chore: fix incorrect rebase * fix: address review feedback * chore: rename utility_process -> utility * chore: update docs * chore: cleanup c++ implemantation * fix: leak in NodeServiceHost impl * chore: minor cleanup * chore: cleanup JS implementation * chore: flip default stdio to inherit * fix: some api improvements * Support cwd option * Remove path restriction for modulePath * Rewire impl for env support * fix: add tests for cwd and env option * chore: alt impl for reading stdio handles * chore: support message queuing * chore: fix lint * chore: new UtilityProcess => utilityProcess.fork * fix: support for uncaught exception exits * chore: remove process.execArgv as default * fix: windows build * fix: style changes * fix: docs and style changes * chore: update patches * spec: disable flaky test on win32 arm CI Co-authored-by: PatchUp <73610968+patchup[bot]@users.noreply.github.com>
Description of Change
Electron applications which run complex Node.js services outside the main process and have necessity to communicate with the renderer process, did it in either of the two ways,
(i) Fork a child process from the renderer via
child_process.fork
and establish a direct communication channel via Node.js sockets.(ii) Fork a child process from the main via
child_process.fork
and establish a direct communication channel via Node.js sockets.If the renderer is sandboxed, then (i) is not possible and with (ii) a direct socket connection cannot be established with the renderer instead messages have to be routed via the main process. In #22404 we tackled the use case of an application using a renderer process in the background to perform cpu intensive work or isolate crash prone components and allowing the capability to communicate with a sandboxed renderer using
MessageChannel
.This PR is inspired by that change and introduces a new module
UtilityProcess
in the main process that allows creating light weight chromium child process with only Node.js integration while also allowing to communicate with a sandboxed renderer usingMessageChannel
. The API was designed based on Node.js child_process.fork to allow for easier transition with one primary difference being that the entry pointmodulePath
must be from within the packaged application to allow only for trusted scripts to be loaded unlike the Node.js version which can load from arbitrary location.Additionally the module upholds the contract that main process is the only trusted process in the application by not allowing to establish communication channels with any renderer in the application by default. Instead the main process is responsible for creating the desired communication channel and sharing the endpoints to the respective child processes.
Example Usage:
main.js
preload.js
worker.js
Future Work:
Allow sandboxing the utility process, it is possible to specify the type of sandbox for the utility process when launching it
content::UtilityProcessHost::SetSandboxType
but it is currently restricted to set of predeclared options. I need to explore how much an embedder can extend the chromium sandbox without any serious patches and think about what level of control we can give to users. An usage example for this would be like, utility process that only performs file I/O can declare a sandbox that disallows network and child_process creating syscalls. Given the complexity of this feature, I don't plan to tackle it in this PR.Would an options to create utility process with just v8 be of any interest ? (i-e) no Node.js integration, no blink integration just vanilla v8
Expose Deno integration with utility process ???
Release Notes
Notes: Added new UtilityProcess API to launch chromium child process with node integration