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

runner.conda: Plug isolation leaks related to Python #311

Merged
merged 2 commits into from
Sep 20, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions CHANGES.md
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,15 @@ development source code and as such may not be routinely kept up to date.

# __NEXT__

## Bug fixes

* We've plugged some isolation leaks in the Conda runtime where the
[Python user site directory](https://docs.python.org/3/library/site.html),
e.g. `~/.local/lib/pythonX.Y/site-packages`, as well as the
[`PYTHONPATH` and `PYTHONHOME` environment variables](https://docs.python.org/3/using/cmdline.html#environment-variables)
could influence and break the runtime.
([#311](https://github.com/nextstrain/cli/pull/311))


# 7.3.0.post1 (19 September 2023)

Expand All @@ -22,6 +31,7 @@ _See also changes in 7.3.0 which was an unreleased version._

* Update CI to test against the SingularityCE 3.x series only ([#314](https://github.com/nextstrain/cli/pull/314))


# 7.3.0 (19 September 2023)

_Unreleased due to [test failures](https://github.com/nextstrain/cli/actions/runs/6238951959). Changes first released as part of 7.3.0.post1._
Expand Down
66 changes: 34 additions & 32 deletions nextstrain/cli/runner/conda.py
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,37 @@
NEXTSTRAIN_BASE = os.environ.get("NEXTSTRAIN_CONDA_BASE_PACKAGE") \
or "nextstrain-base"

PYTHONUSERBASE = RUNTIME_ROOT / "python-user-base"

# Construct a PATH with our runtime prefix which provides some, but not total,
# isolation from the rest of the system.
PATH = os.pathsep.join(map(str, [
# Programs installed by this runtime.
PREFIX_BIN,

# Python's idea of a default path for the system, which currently under
# CPython is either "/bin:/usr/bin" on POSIX systems or ".;C:\\bin" on
# Windows. This will ensure basic system commands like `ls` are
# available, although it will also "leak" any user-installed programs
# there.
os.defpath,
]))

EXEC_ENV = {
"PATH": PATH,

# Avoid letting user-set custom Python installs and module search paths
# from outside the runtime leak inside.
"PYTHONHOME": None,
"PYTHONPATH": None,

# Avoid letting the user site directory leak into the runtime, c.f.
# <https://docs.python.org/3/library/site.html> and
# <https://docs.python.org/3/using/cmdline.html#envvar-PYTHONNOUSERSITE>.
"PYTHONUSERBASE": str(PYTHONUSERBASE),
"PYTHONNOUSERSITE": "1",
}


def register_arguments(parser) -> None:
"""
Expand Down Expand Up @@ -136,36 +167,7 @@ def run(opts, argv, working_volume = None, extra_env: Env = {}, cpus: int = None
# DLL searching).
# -trs, 13 Jan 2023

extra_env = {
**extra_env,
"PATH": path_with_prefix(),
}

return exec_or_return(argv, extra_env)


def path_with_prefix() -> str:
"""
Constructs a ``PATH`` with our runtime prefix.

The returned ``PATH`` consists of the:

1. Runtime prefix
2. :py:attr:`os.defpath`

which provides some, but not total, isolation from the rest of the system.
"""
return os.pathsep.join(map(str, [
# Programs installed by this runtime.
PREFIX_BIN,

# Python's idea of a default path for the system, which currently under
# CPython is either "/bin:/usr/bin" on POSIX systems or ".;C:\\bin" on
# Windows. This will ensure basic system commands like `ls` are
# available, although it will also "leak" any user-installed programs
# there.
os.defpath,
]))
return exec_or_return(argv, {**extra_env, **EXEC_ENV})


def setup(dry_run: bool = False, force: bool = False) -> RunnerSetupStatus:
Expand Down Expand Up @@ -395,7 +397,7 @@ def which_finds_our(cmd) -> bool:
# ".exe" extension on Windows, which is why we don't just naively test
# for existence ourselves. File extensions are also why we don't test
# equality below instead check containment in PREFIX_BIN.
found = shutil.which(cmd, path = path_with_prefix())
found = shutil.which(cmd, path = PATH)

if not found:
return False
Expand All @@ -412,7 +414,7 @@ def which_finds_our(cmd) -> bool:

def runnable(*argv) -> bool:
try:
capture_output(argv, extra_env = {"PATH": path_with_prefix()})
capture_output(argv, extra_env = EXEC_ENV)
return True
except (OSError, subprocess.CalledProcessError):
return False
Expand Down
Loading