javascript: Use binary shims in nodejs sandboxes (pantsbuild#18682)

Use `BinaryShims` instead of adding "/bin" to PATH in the nodejs
sandbox.

Addresses pantsbuild#18609,
pantsbuild#18597

src/python/pants/backend/docker/subsystems/docker_options.py

@@ -3,12 +3,11 @@
 
 from __future__ import annotations
 
-import os
 import sys
 from typing import Any
 
 from pants.backend.docker.registries import DockerRegistries
-from pants.engine.env_vars import EnvironmentVars
+from pants.core.util_rules.search_paths import ExecutableSearchPathsOptionMixin
 from pants.option.option_types import (
     BoolOption,
     DictOption,
@@ -20,7 +19,6 @@
 from pants.option.subsystem import Subsystem
 from pants.util.docutil import bin_name
 from pants.util.memo import memoized_method
-from pants.util.ordered_set import OrderedSet
 from pants.util.strutil import bullet_list, softwrap
 
 doc_links = {
@@ -34,7 +32,7 @@ class DockerOptions(Subsystem):
     options_scope = "docker"
     help = "Options for interacting with Docker."
 
-    class EnvironmentAware:
+    class EnvironmentAware(ExecutableSearchPathsOptionMixin, Subsystem.EnvironmentAware):
         _env_vars = ShellStrListOption(
             help=softwrap(
                 """
@@ -46,36 +44,16 @@ class EnvironmentAware:
             ),
             advanced=True,
         )
-        _executable_search_paths = StrListOption(
-            default=["<PATH>"],
-            help=softwrap(
-                """
+        executable_search_paths_help = softwrap(
+            """
             The PATH value that will be used to find the Docker client and any tools required.
-
-            The special string `"<PATH>"` will expand to the contents of the PATH env var.
             """
-            ),
-            advanced=True,
-            metavar="<binary-paths>",
         )
 
         @property
         def env_vars(self) -> tuple[str, ...]:
             return tuple(sorted(set(self._env_vars)))
 
-        @memoized_method
-        def executable_search_path(self, env: EnvironmentVars) -> tuple[str, ...]:
-            def iter_path_entries():
-                for entry in self._executable_search_paths:
-                    if entry == "<PATH>":
-                        path = env.get("PATH")
-                        if path:
-                            yield from path.split(os.pathsep)
-                    else:
-                        yield entry
-
-            return tuple(OrderedSet(iter_path_entries()))
-
     _registries = DictOption[Any](
         help=softwrap(
             """

src/python/pants/backend/docker/util_rules/docker_binary.py

@@ -18,7 +18,6 @@
     BinaryShims,
     BinaryShimsRequest,
 )
-from pants.engine.env_vars import EnvironmentVars, EnvironmentVarsRequest
 from pants.engine.fs import Digest
 from pants.engine.process import Process, ProcessCacheScope
 from pants.engine.rules import Get, collect_rules, rule
@@ -137,8 +136,7 @@ async def find_docker(_: DockerBinaryRequest, docker: DockerBinary) -> DockerBin
 async def get_docker(
     docker_options: DockerOptions, docker_options_env_aware: DockerOptions.EnvironmentAware
 ) -> DockerBinary:
-    env = await Get(EnvironmentVars, EnvironmentVarsRequest(["PATH"]))
-    search_path = docker_options_env_aware.executable_search_path(env)
+    search_path = docker_options_env_aware.executable_search_path
     request = BinaryPathRequest(
         binary_name="docker",
         search_path=search_path,

src/python/pants/backend/javascript/dependency_inference/import_parser/rules_test.py

@@ -25,14 +25,16 @@
 
 @pytest.fixture
 def rule_runner() -> RuleRunner:
-    return RuleRunner(
+    rule_runner = RuleRunner(
         rules=[
             *import_parser_rules(),
             QueryRule(JSImportStrings, (ParseJsImportStrings,)),
             QueryRule(InstalledJavascriptImportParser, ()),
         ],
         target_types=[JSSourceTarget, JSSourcesGeneratorTarget],
     )
+    rule_runner.set_options([], env_inherit={"PATH"})
+    return rule_runner
 
 
 def test_installs_parser_modules(rule_runner: RuleRunner) -> None:

src/python/pants/backend/javascript/dependency_inference/rules_test.py

@@ -27,7 +27,7 @@
 
 @pytest.fixture
 def rule_runner() -> RuleRunner:
-    return RuleRunner(
+    rule_runner = RuleRunner(
         rules=[
             *package_json.rules(),
             *dependency_inference_rules(),
@@ -38,6 +38,8 @@ def rule_runner() -> RuleRunner:
         ],
         target_types=[*package_json.target_types(), JSSourceTarget, JSSourcesGeneratorTarget],
     )
+    rule_runner.set_options([], env_inherit={"PATH"})
+    return rule_runner
 
 
 def given_package(name: str, version: str, **kwargs: str | dict[str, str]) -> str:

src/python/pants/backend/javascript/goals/lockfile_test.py

@@ -32,7 +32,7 @@
 
 @pytest.fixture
 def rule_runner() -> RuleRunner:
-    return RuleRunner(
+    rule_runner = RuleRunner(
         rules=[
             *lockfile.rules(),
             QueryRule(
@@ -53,6 +53,8 @@ def rule_runner() -> RuleRunner:
         ],
         target_types=[PackageJsonTarget],
     )
+    rule_runner.set_options([], env_inherit={"PATH"})
+    return rule_runner
 
 
 def given_package_with_name(name: str) -> str:

src/python/pants/backend/javascript/goals/test_integration_test.py

@@ -40,6 +40,7 @@ def rule_runner() -> RuleRunner:
         ],
         objects=dict(package_json.build_file_aliases().objects),
     )
+    rule_runner.set_options([], env_inherit={"PATH"})
     return rule_runner
 
 
@@ -157,7 +158,7 @@ def test_mocha_tests_are_successful(rule_runner: RuleRunner) -> None:
 
 
 def test_jest_test_with_coverage_reporting(rule_runner: RuleRunner) -> None:
-    rule_runner.set_options(args=["--test-use-coverage", "True"])
+    rule_runner.set_options(args=["--test-use-coverage", "True"], env_inherit={"PATH"})
     rule_runner.write_files(
         {
             "foo/BUILD": textwrap.dedent(

src/python/pants/backend/javascript/package/rules_integration_test.py

@@ -26,7 +26,7 @@
 
 @pytest.fixture
 def rule_runner() -> RuleRunner:
-    return RuleRunner(
+    rule_runner = RuleRunner(
         rules=[
             *package_rules(),
             QueryRule(BuiltPackage, (NodePackageTarFieldSet,)),
@@ -41,6 +41,8 @@ def rule_runner() -> RuleRunner:
         ],
         objects=dict(package_json.build_file_aliases().objects),
     )
+    rule_runner.set_options([], env_inherit={"PATH"})
+    return rule_runner
 
 
 def test_packages_sources_as_resource_using_build_tool(rule_runner: RuleRunner) -> None:

src/python/pants/backend/javascript/package/rules_test.py

@@ -27,7 +27,7 @@
 
 @pytest.fixture
 def rule_runner() -> RuleRunner:
-    return RuleRunner(
+    rule_runner = RuleRunner(
         rules=[
             *package_rules(),
             QueryRule(BuiltPackage, (NodePackageTarFieldSet,)),
@@ -42,6 +42,8 @@ def rule_runner() -> RuleRunner:
         ],
         objects=dict(package_json.build_file_aliases().objects),
     )
+    rule_runner.set_options([], env_inherit={"PATH"})
+    return rule_runner
 
 
 def test_creates_tar_for_package_json(rule_runner: RuleRunner) -> None:

src/python/pants/backend/javascript/run/rules_test.py

@@ -19,7 +19,7 @@
 
 @pytest.fixture
 def rule_runner() -> RuleRunner:
-    return RuleRunner(
+    rule_runner = RuleRunner(
         rules=[
             *run_rules(),
             QueryRule(RunRequest, (RunNodeBuildScriptFieldSet,)),
@@ -31,6 +31,8 @@ def rule_runner() -> RuleRunner:
         ],
         objects=dict(package_json.build_file_aliases().objects),
     )
+    rule_runner.set_options([], env_inherit={"PATH"})
+    return rule_runner
 
 
 def test_creates_run_requests_package_json_scripts(rule_runner: RuleRunner) -> None:

src/python/pants/backend/javascript/subsystems/nodejs.py

@@ -23,6 +23,7 @@
 )
 from pants.core.util_rules.external_tool import rules as external_tool_rules
 from pants.core.util_rules.search_paths import (
+    ExecutableSearchPathsOptionMixin,
     ValidatedSearchPaths,
     ValidateSearchPathsRequest,
     VersionManagerSearchPaths,
@@ -34,6 +35,8 @@
     BinaryPathRequest,
     BinaryPaths,
     BinaryPathTest,
+    BinaryShims,
+    BinaryShimsRequest,
 )
 from pants.engine.env_vars import EnvironmentVars, EnvironmentVarsRequest, PathEnvironmentVariable
 from pants.engine.fs import EMPTY_DIGEST, Digest, DownloadFile
@@ -117,9 +120,7 @@ async def download_known_version(
         download_file = DownloadFile(url, FileDigest(known_version.sha256, known_version.filesize))
         return await Get(DownloadedExternalTool, ExternalToolRequest(download_file, exe))
 
-    class EnvironmentAware(Subsystem.EnvironmentAware):
-        env_vars_used_by_options = ("PATH",)
-
+    class EnvironmentAware(ExecutableSearchPathsOptionMixin, Subsystem.EnvironmentAware):
         search_path = StrListOption(
             default=["<PATH>"],
             help=lambda cls: help_text(
@@ -154,6 +155,12 @@ class EnvironmentAware(Subsystem.EnvironmentAware):
             metavar="<binary-paths>",
         )
 
+        executable_search_paths_help = softwrap(
+            """
+            The PATH value that will be used to find any tools required to run nodejs processes.
+            """
+        )
+
 
 class UserChosenNodeJSResolveAliases(FrozenDict[str, str]):
     pass
@@ -235,12 +242,13 @@ class NodeJSBinaries:
 class NodeJSProcessEnvironment:
     binaries: NodeJSBinaries
     npm_config_cache: str
+    tool_binaries: BinaryShims
 
     base_bin_dir: ClassVar[str] = "__node"
 
     def to_env_dict(self) -> dict[str, str]:
         return {
-            "PATH": f"/bin:{self.binary_directory}",
+            "PATH": f"{self.tool_binaries.path_component}:{self.binary_directory}",
             "npm_config_cache": self.npm_config_cache,  # Normally stored at ~/.npm
         }
 
@@ -253,12 +261,32 @@ def binary_directory(self) -> str:
         return self.binaries.binary_dir
 
     def immutable_digest(self) -> dict[str, Digest]:
-        return {self.base_bin_dir: self.binaries.digest} if self.binaries.digest else {}
+        return (
+            {self.base_bin_dir: self.binaries.digest, **self.tool_binaries.immutable_input_digests}
+            if self.binaries.digest
+            else {**self.tool_binaries.immutable_input_digests}
+        )
 
 
 @rule(level=LogLevel.DEBUG)
-async def node_process_environment(binaries: NodeJSBinaries) -> NodeJSProcessEnvironment:
-    return NodeJSProcessEnvironment(binaries=binaries, npm_config_cache="._npm")
+async def node_process_environment(
+    binaries: NodeJSBinaries, nodejs: NodeJS.EnvironmentAware
+) -> NodeJSProcessEnvironment:
+    binary_shims = await Get(
+        BinaryShims,
+        BinaryShimsRequest.for_binaries(
+            "sh",
+            "bash",
+            "mkdir",  # Some default scripts are generated using mkdir, rm & touch.
+            "rm",
+            "touch",
+            rationale="execute a nodejs process",
+            search_path=nodejs.executable_search_path,
+        ),
+    )
+    return NodeJSProcessEnvironment(
+        binaries=binaries, npm_config_cache="._npm", tool_binaries=binary_shims
+    )
 
 
 @dataclass(frozen=True)

src/python/pants/backend/javascript/subsystems/nodejs_test.py

@@ -43,7 +43,7 @@
 
 @pytest.fixture
 def rule_runner() -> RuleRunner:
-    return RuleRunner(
+    rule_runner = RuleRunner(
         rules=[
             *nodejs.rules(),
             *source_files.rules(),
@@ -55,6 +55,8 @@ def rule_runner() -> RuleRunner:
         ],
         target_types=[JSSourcesGeneratorTarget],
     )
+    rule_runner.set_options([], env_inherit={"PATH"})
+    return rule_runner
 
 
 def test_npx_process(rule_runner: RuleRunner):

src/python/pants/backend/javascript/subsystems/npx_tool_test.py

@@ -30,7 +30,7 @@ def rule_runner() -> RuleRunner:
 
 
 def test_version_option_overrides_default(rule_runner: RuleRunner):
-    rule_runner.set_options(["[email protected]"])
+    rule_runner.set_options(["[email protected]"], env_inherit={"PATH"})
     tool = rule_runner.request(CowsayTool, [])
     assert tool.default_version == "[email protected]"
     assert tool.version == "[email protected]"

src/python/pants/backend/shell/subsystems/shell_setup.py

@@ -3,12 +3,9 @@
 
 from __future__ import annotations
 
-import os
-
-from pants.option.option_types import BoolOption, StrListOption
+from pants.core.util_rules.search_paths import ExecutableSearchPathsOptionMixin
+from pants.option.option_types import BoolOption
 from pants.option.subsystem import Subsystem
-from pants.util.memo import memoized_property
-from pants.util.ordered_set import OrderedSet
 from pants.util.strutil import softwrap
 
 
@@ -31,32 +28,10 @@ class ShellSetup(Subsystem):
         advanced=True,
     )
 
-    class EnvironmentAware(Subsystem.EnvironmentAware):
-        env_vars_used_by_options = ("PATH",)
-
-        _executable_search_path = StrListOption(
-            default=["<PATH>"],
-            help=softwrap(
-                """
-                The PATH value that will be used to find shells and to run certain processes
-                like the shunit2 test runner.
-
-                The special string `"<PATH>"` will expand to the contents of the PATH env var.
-                """
-            ),
-            advanced=True,
-            metavar="<binary-paths>",
+    class EnvironmentAware(ExecutableSearchPathsOptionMixin, Subsystem.EnvironmentAware):
+        executable_search_paths_help = softwrap(
+            """
+            The PATH value that will be used to find shells
+            and to run certain processes like the shunit2 test runner.
+            """
         )
-
-        @memoized_property
-        def executable_search_path(self) -> tuple[str, ...]:
-            def iter_path_entries():
-                for entry in self._executable_search_path:
-                    if entry == "<PATH>":
-                        path = self._options_env.get("PATH")
-                        if path:
-                            yield from path.split(os.pathsep)
-                    else:
-                        yield entry
-
-            return tuple(OrderedSet(iter_path_entries()))