fix: Use -P to enable safe path semantics instead of PYTHONSAFEPATH

CHANGELOG.md

@@ -26,6 +26,12 @@ A brief description of the categories of changes:
 
 ### Changed
 * (gazelle): Update error messages when unable to resolve a dependency to be more human-friendly.
+* (rules) The `PYTHONSAFEPATH` environment variable is no longer used to enable safe-path mode
+  since it is unintentionally inherited by subprocesses. Instead, `-P` is passed to the runtime
+  interpreter given its version can be detected and is greater than or equal to 3.11. If the 
+  runtime interpreter can not be detected or the version is less than 3.11, safe-path mode is not
+  enabled.
+  ([#2060](https://github.com/bazelbuild/rules_python/issues/2060)).
 
 ### Fixed
 * (whl_library): Remove `--no-index` and add `--no-build-isolation` to the

python/private/common/py_executable_bazel.bzl

@@ -168,6 +168,19 @@ def _create_executable(
         runfiles_details):
     _ = is_test, cc_details, native_deps_details  # @unused
 
+    interpreter_opts = ""
+
+    # If the runtime interpreter has been detected and its version is 3.11 or higher, enable safe-path mode
+    # by passing the -P argument to the interpreter
+    if hasattr(runtime_details.effective_runtime, "interpreter_version_info"):
+        interpreter_version_info = runtime_details.effective_runtime.interpreter_version_info
+        if interpreter_version_info and \
+           interpreter_version_info.major and \
+           interpreter_version_info.major >= 3 and \
+           interpreter_version_info.minor and \
+           interpreter_version_info.minor >= 11:
+            interpreter_opts += " -P"
+
     is_windows = target_platform_has_any_constraint(ctx, ctx.attr._windows_constraints)
 
     if is_windows:
@@ -207,6 +220,7 @@ def _create_executable(
             output = zip_main,
             main_py = main_py,
             imports = imports,
+            interpreter_opts = interpreter_opts,
             is_for_zip = True,
             runtime_details = runtime_details,
         )
@@ -270,6 +284,7 @@ def _create_executable(
             ctx,
             output = executable,
             zip_file = zip_file,
+            interpreter_opts = interpreter_opts,
             stage2_bootstrap = stage2_bootstrap,
             runtime_details = runtime_details,
         )
@@ -281,6 +296,7 @@ def _create_executable(
             runtime_details = runtime_details,
             is_for_zip = False,
             imports = imports,
+            interpreter_opts = interpreter_opts,
             main_py = main_py,
         )
     else:
@@ -368,11 +384,13 @@ def _create_stage1_bootstrap(
         main_py = None,
         stage2_bootstrap = None,
         imports = None,
+        interpreter_opts,
         is_for_zip,
         runtime_details):
     runtime = runtime_details.effective_runtime
 
     subs = {
+        "%interpreter_opts%": interpreter_opts,
         "%is_zipfile%": "1" if is_for_zip else "0",
         "%python_binary%": runtime_details.executable_interpreter_path,
         "%target%": str(ctx.label),
@@ -522,7 +540,7 @@ def _get_zip_runfiles_path(path, workspace_name, legacy_external_runfiles):
         zip_runfiles_path = paths.normalize("{}/{}".format(workspace_name, path))
     return "{}/{}".format(_ZIP_RUNFILES_DIRECTORY_NAME, zip_runfiles_path)
 
-def _create_executable_zip_file(ctx, *, output, zip_file, stage2_bootstrap, runtime_details):
+def _create_executable_zip_file(ctx, *, output, zip_file, interpreter_opts, stage2_bootstrap, runtime_details):
     prelude = ctx.actions.declare_file(
         "{}_zip_prelude.sh".format(output.basename),
         sibling = output,
@@ -533,6 +551,7 @@ def _create_executable_zip_file(ctx, *, output, zip_file, stage2_bootstrap, runt
             output = prelude,
             stage2_bootstrap = stage2_bootstrap,
             runtime_details = runtime_details,
+            interpreter_opts = interpreter_opts,
             is_for_zip = True,
         )
     else:

python/private/python_bootstrap_template.txt

@@ -32,6 +32,9 @@ if IsRunningFromZip():
 else:
   import re
 
+def GetInterpreterOpts():
+  return "%interpreter_opts%".split()
+
 # Return True if running on Windows
 def IsWindows():
   return os.name == 'nt'
@@ -363,8 +366,9 @@ def ExecuteFile(python_program, main_filename, args, env, module_space,
     ret_code = _RunForCoverage(python_program, main_filename, args, env,
                                coverage_entrypoint, workspace)
   else:
+    subprocess_argv = [python_program] + GetInterpreterOpts() + [main_filename] + args
     ret_code = subprocess.call(
-      [python_program, main_filename] + args,
+      subprocess_argv,
       env=env,
       cwd=workspace
     )
@@ -381,7 +385,7 @@ def _RunExecv(python_program, main_filename, args, env):
   """Executes the given Python file using the various environment settings."""
   os.environ.update(env)
   PrintVerbose("RunExecv: environ:", os.environ)
-  argv = [python_program, main_filename] + args
+  argv = [python_program] + GetInterpreterOpts() + [main_filename] + args
   PrintVerbose("RunExecv: argv:", python_program, argv)
   os.execv(python_program, argv)
 
@@ -410,16 +414,16 @@ relative_files = True
   PrintVerboseCoverage('Coverage entrypoint:', coverage_entrypoint)
   # First run the target Python file via coveragepy to create a .coverage
   # database file, from which we can later export lcov.
+  subprocess_argv = [python_program] + GetInterpreterOpts() + [
+    coverage_entrypoint,
+    "run",
+    "--rcfile=" + rcfile_name,
+    "--append",
+    "--branch",
+    main_filename
+  ] + args
   ret_code = subprocess.call(
-    [
-      python_program,
-      coverage_entrypoint,
-      "run",
-      "--rcfile=" + rcfile_name,
-      "--append",
-      "--branch",
-      main_filename
-    ] + args,
+    subprocess_argv,
     env=env,
     cwd=workspace
   )
@@ -495,10 +499,6 @@ def Main():
   if runfiles_envkey:
     new_env[runfiles_envkey] = runfiles_envvalue
 
-  # Don't prepend a potentially unsafe path to sys.path
-  # See: https://docs.python.org/3.11/using/cmdline.html#envvar-PYTHONSAFEPATH
-  new_env['PYTHONSAFEPATH'] = '1'
-
   main_filename = os.path.join(module_space, main_rel_path)
   main_filename = GetWindowsPathWithUNCPrefix(main_filename)
   assert os.path.exists(main_filename), \

python/private/stage1_bootstrap_template.sh

@@ -15,6 +15,9 @@ PYTHON_BINARY='%python_binary%'
 # 0 or 1
 IS_ZIPFILE="%is_zipfile%"
 
+# command line options to be passed to the interpreter
+INTERPRETER_OPTS="%interpreter_opts%"
+
 if [[ "$IS_ZIPFILE" == "1" ]]; then
   # NOTE: Macs have an old version of mktemp, so we must use only the
   # minimal functionality of it.
@@ -102,18 +105,9 @@ stage2_bootstrap="$RUNFILES_DIR/$STAGE2_BOOTSTRAP"
 declare -a interpreter_env
 declare -a interpreter_args
 
-# Don't prepend a potentially unsafe path to sys.path
-# See: https://docs.python.org/3.11/using/cmdline.html#envvar-PYTHONSAFEPATH
-# NOTE: Only works for 3.11+
-# We inherit the value from the outer environment in case the user wants to
-# opt-out of using PYTHONSAFEPATH. To opt-out, they have to set
-# `PYTHONSAFEPATH=` (empty string). This is because Python treats the empty
-# value as false, and any non-empty value as true.
-# ${FOO+WORD} expands to empty if $FOO is undefined, and WORD otherwise.
-if [[ -z "${PYTHONSAFEPATH+x}" ]]; then
-  # ${FOO-WORD} expands to WORD if $FOO is undefined, and $FOO otherwise
-  interpreter_env+=("PYTHONSAFEPATH=${PYTHONSAFEPATH-1}")
-fi
+# Split the space-separated values in INTERPRETER_OPTS and store them
+# inside the array interpreter_args
+read -ra interpreter_args <<< "$INTERPRETER_OPTS"
 
 if [[ "$IS_ZIPFILE" == "1" ]]; then
   interpreter_args+=("-XRULES_PYTHON_ZIP_DIR=$zip_dir")

python/private/zip_main_template.py

@@ -27,12 +27,13 @@
 _PYTHON_BINARY = "%python_binary%"
 _WORKSPACE_NAME = "%workspace_name%"
 
+def get_interpreter_opts():
+  return "%interpreter_opts%".split()
 
 # Return True if running on Windows
 def is_windows():
     return os.name == "nt"
 
-
 def get_windows_path_with_unc_prefix(path):
     """Adds UNC prefix after getting a normalized absolute Windows path.
 
@@ -208,7 +209,7 @@ def execute_file(
     # - When running in a workspace or zip file, we need to clean up the
     #   workspace after the process finishes so control must return here.
     try:
-        subprocess_argv = [python_program, main_filename] + args
+        subprocess_argv = [python_program] + get_interpreter_opts() + [main_filename] + args
         print_verbose("subprocess argv:", values=subprocess_argv)
         print_verbose("subprocess env:", mapping=env)
         print_verbose("subprocess cwd:", workspace)
@@ -244,10 +245,6 @@ def main():
 
     new_env["RUNFILES_DIR"] = module_space
 
-    # Don't prepend a potentially unsafe path to sys.path
-    # See: https://docs.python.org/3.11/using/cmdline.html#envvar-PYTHONSAFEPATH
-    new_env["PYTHONSAFEPATH"] = "1"
-
     main_filename = os.path.join(module_space, main_rel_path)
     main_filename = get_windows_path_with_unc_prefix(main_filename)
     assert os.path.exists(main_filename), (

tests/base_rules/BUILD.bazel

@@ -72,9 +72,9 @@ py_reconfig_test(
 )
 
 sh_py_run_test(
-    name = "inherit_pythonsafepath_env_test",
+    name = "pythonsafepath_test",
     bootstrap_impl = "script",
     py_src = "bin.py",
-    sh_src = "inherit_pythonsafepath_env_test.sh",
+    sh_src = "pythonsafepath_test.sh",
     target_compatible_with = _SUPPORTS_BOOTSTRAP_SCRIPT,
 )

tests/base_rules/inherit_pythonsafepath_env_test.sh → tests/base_rules/pythonsafepath_test.sh

@@ -45,25 +45,12 @@ function expect_match() {
   fi
 }
 
-
-echo "Check inherited and disabled"
-# Verify setting it to empty string disables safe path
-actual=$(PYTHONSAFEPATH= $bin 2>&1)
-expect_match "sys.flags.safe_path: False" "$actual"
-expect_match "PYTHONSAFEPATH: EMPTY" "$actual"
-
-echo "Check inherited and propagated"
-# Verify setting it to any string enables safe path and that
-# value is propagated
-actual=$(PYTHONSAFEPATH=OUTER $bin 2>&1)
-expect_match "sys.flags.safe_path: True" "$actual"
-expect_match "PYTHONSAFEPATH: OUTER" "$actual"
-
-echo "Check enabled by default"
-# Verifying doing nothing leaves safepath enabled by default
+echo "Check PYTHONSAFEPATH is unset while safe_path is True by default"
+# Verify that safe_path is True despite PYTHONSAFEPATH being UNSET (since we
+# use the -P argument to set it)
 actual=$($bin 2>&1)
 expect_match "sys.flags.safe_path: True" "$actual"
-expect_match "PYTHONSAFEPATH: 1" "$actual"
+expect_match "PYTHONSAFEPATH: UNSET" "$actual"
 
 # Exit if any of the expects failed
 [[ ! -e EXPECTATION_FAILED ]]