fix(pathfinder): make find_nvidia_binary_utility deterministic, never search CWD by aryanputta · Pull Request #2196 · NVIDIA/cuda-python

aryanputta · 2026-06-10T23:21:21Z

Summary

find_nvidia_binary_utility previously delegated to shutil.which(name, path=trusted_dirs). On Windows, CPython's shutil.which prepends the process current working directory even when an explicit path= is supplied. That allowed a binary in an arbitrary CWD to shadow the CUDA / Conda / wheel binary that pathfinder is meant to discover through a bounded search.

This PR replaces that delegation with an explicit resolver over trusted directories only, then folds in the maintainer-requested CTK-root canary fallback so users who follow the CUDA installation guide continue to work when the toolkit is visible through the dynamic loader.

Fix

The binary lookup order is now:

NVIDIA wheel bin/ directories from site-packages.
CONDA_PREFIX.
CUDA_HOME / CUDA_PATH.
CTK-root canary fallback: resolve cudart through the OS dynamic loader, derive the CUDA Toolkit root, and search that root's bin layout.

The current working directory and ambient PATH are never consulted.

Notes:

POSIX execute-bit semantics and Windows executable-extension semantics are preserved.
Empty and duplicate trusted directories are skipped.
The canary fallback only runs after the explicit trusted directories miss.
The canary path reuses the dynamic-library CTK-root discovery helper, so library and binary discovery share the same root derivation.

Tests

Reworked search-path tests to assert deterministic probe order.
Added CWD-isolation coverage for the core find_nvidia_binary_utility() violates deterministic-search contract on Windows (CWD leakage via shutil.which) #2119 regression.
Added first-match, empty/duplicate directory, and POSIX non-executable rejection coverage.
Added CTK-root canary fallback coverage, including Windows bin/x64, bin/x86_64, and bin layout.
Added coverage that the canary is not consulted when an earlier trusted directory hits.
Added focused tests for shared resolve_ctk_root_via_canary(...).

Local checks: ruff check and ruff format --check pass on the touched files.

… search CWD find_nvidia_binary_utility assembled a bounded list of trusted directories (NVIDIA wheel bin/, CONDA_PREFIX, CUDA_HOME/CUDA_PATH) and then delegated to shutil.which(name, path=trusted_dirs). On Windows shutil.which prepends the process current working directory to the search even when an explicit path= is supplied, so a binary located in an arbitrary (possibly attacker-writable) CWD could be returned in preference to the trusted CUDA / Conda / wheel binary. That violates the pathfinder contract of a deterministic lookup over a documented, bounded set of trusted roots. Replace the shutil.which delegation with an explicit resolver that searches only the trusted directories, in order, returning the first executable match. The current working directory and ambient PATH are never consulted. POSIX execute-bit (X_OK) and Windows extension semantics are preserved, so behavior is unchanged except for removing the CWD/PATH leakage. Names resolved in the existing trusted dirs return exactly as before. Rewrites the search-path tests to assert the deterministic probe order and adds TestResolveInTrustedDirs covering CWD isolation, first-match-wins, empty/duplicate dir skipping, and POSIX non-executable rejection. Fixes NVIDIA#2119

copy-pr-bot · 2026-06-10T23:21:25Z

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

rwgk · 2026-06-10T23:53:41Z

Hi @aryanputta, this is great, but in isolation it'll probably break some users. This was pointed out by @kkraus14 here before:

[codex] Avoid untrusted binary lookup paths #2111 (comment)

If someone follows this installation guide and sets LD_LIBRARY_PATH for libraries and PATH for executables ...

I ran the context and my thoughts by GPT around the same time, which lead to the conclusion at the end of this comment:

[codex] Avoid untrusted binary lookup paths #2111 (comment)

Use a custom bounded finder, add CTK-root canary fallback, and do not implicitly search PATH.

I still think that's the right compromise:

We will find the binary utility via LD_LIBRARY_PATH if set. It's still an attack vector, but significantly more difficult to exploit than hacking PATH.
Users following the installation guide will not see a difference in most cases, and get a more self-consistent outcome in the rare case that the searches via PATH and LD_LIBRARY_PATH produce diverging results.

The behavior difference is significant enough though that I'd want to bump the pathfinder minor version, which we can do; I don't have concerns about that.

How do you feel about expanding this PR to fold in the CTK-root canary fallback? — I believe the code changes will still be quite modest.

…utility After the deterministic search over the explicit trusted directories (NVIDIA wheel bin/, CONDA_PREFIX, CUDA_HOME/CUDA_PATH) misses, fall back to a CTK-root canary probe: resolve cudart through the OS dynamic loader, which honors LD_LIBRARY_PATH on Linux and the native DLL search on Windows, derive the CUDA Toolkit root from its absolute path, and search that root's bin layout. This addresses the concern raised on NVIDIA#2196: users who follow the CUDA Linux installation guide set LD_LIBRARY_PATH for libraries and PATH for executables. The bounded finder alone would stop finding the utility for them because PATH is intentionally never consulted. The canary fallback recovers that case through LD_LIBRARY_PATH instead of PATH. LD_LIBRARY_PATH is still an attack vector, but a significantly harder one to exploit than PATH, and the ambient PATH and process CWD remain unused. The canary runs only after the explicit trusted dirs miss, so the common wheel/conda/CUDA_HOME cases never spawn the resolver subprocess. The canary -> CTK-root resolution is factored into a shared resolve_ctk_root_via_canary helper reused by the dynamic-library CTK-root canary flow. Adds tests for the fallback (found, ordering, Windows bin layout, not consulted when found earlier, cached) and for resolve_ctk_root_via_canary. Adds 1.6.0 release notes for the minor version bump.

aryanputta · 2026-06-11T01:12:03Z

Thanks @rwgk, that compromise makes sense and I've folded it in (pushed c3d39f0, no force-push). Agreed on the reasoning: the bounded finder shouldn't regress users who follow the Linux install guide and only set LD_LIBRARY_PATH + PATH, but reaching them via LD_LIBRARY_PATH instead of PATH is the safer trade.

What the new commit adds

Search order is unchanged for the explicit trusted dirs, with the canary as a strict fallback:

NVIDIA wheel bin/ (site-packages)
CONDA_PREFIX
CUDA_HOME / CUDA_PATH
CTK-root canary (new): resolve cudart through the OS dynamic loader, derive the CTK root from its absolute path, search that root's bin layout

Key points:

The canary runs only when steps 1-3 miss, so the common wheel/conda/CUDA_HOME cases never spawn the resolver subprocess.
It reuses the existing canary toolbox. I factored the cudart -> CTK-root resolution into resolve_ctk_root_via_canary(...) in load_nvidia_dynamic_lib.py and had the existing dynamic-library _try_ctk_root_canary call it too, so lib and binary discovery share one path. The resolver runs in the isolated subprocess that honors LD_LIBRARY_PATH on Linux and the native DLL search on Windows.
PATH and the process CWD are still never consulted, so the LD_LIBRARY_PATH surface is the only added vector, exactly as you described.

Version

Added docs/source/release/1.6.0-notes.rst for the minor bump (version is tag-driven, so this is the doc side of it). Happy to renumber if you'd rather it land under a different version.

Tests

Added coverage for the fallback in tests/test_find_nvidia_binaries.py (found via canary, ordering vs CUDA_HOME, Windows bin/x64/bin/x86_64/bin layout, not consulted when an earlier dir hits, cached) and a focused resolve_ctk_root_via_canary unit test in tests/test_ctk_root_discovery.py. ruff check and ruff format --check are clean.

pre-commit.ci mypy flagged returning Any from resolve_ctk_root_via_canary and _resolve_ctk_root_via_canary (both declared -> str | None), because derive_ctk_root resolves to Any under the pathfinder mypy config. Bind the result to an annotated local before returning, matching the pattern used elsewhere in the package.

aryanputta · 2026-06-23T23:55:06Z

Quick check-in: is there anything else you would like changed here after the CTK-root canary fallback update, or is this mainly waiting on review / runner validation now? Happy to adjust if the version note or fallback behavior should be shaped differently.

rwgk

Generally, in private functions I'd add docstrings only to explain things that are not already obvious. Simple comments are often most efficient.

rwgk · 2026-06-12T18:31:25Z

+# CUDA Toolkit canary library used to derive the toolkit root when it is only
+# visible through the dynamic loader. ``cudart`` always ships with the CTK and
+# matches the anchor used by the dynamic-library CTK-root canary flow.
+_CTK_ROOT_CANARY_ANCHOR_LIBNAME = "cudart"


Headers, dynamic-library descriptors, and now binary discovery all rely on the same "cudart" choice. Could you please centralize the anchor in a neutral shared module, e.g. _utils/ctk_root_canary.py?

I think this will go well with the existing code:

CTK_ROOT_CANARY_ANCHOR_LIBNAMES = ("cudart",)

rwgk · 2026-06-12T18:32:46Z

+    On Windows the CTK ships binaries under ``bin/x64`` (CTK 13), ``bin/x86_64``,
+    and ``bin`` (CTK 12); on Linux they live in ``bin``.


Please remove: it essentially just explains the code immediately below, which is actually easier to read than this prose; and it's a private helper function.

rwgk · 2026-06-12T18:34:27Z

+    ``cudart`` is resolved by the OS dynamic loader, which honors
+    ``LD_LIBRARY_PATH`` on Linux and the native DLL search on Windows, and the
+    toolkit root is derived from its absolute path. The ambient ``PATH`` is
+    never consulted. The loader module is imported lazily to avoid pulling the


Please remove the first two sentences here: they only explain code elsewhere; for a private helper function that's more noise than signal.

Also throughout: Please avoid all mentions like "unlike shutil.which" and "ambient PATH". That only makes sense in the context of this particular PR, but will be puzzling later. Explaining the which/PATH/CWD pitfalls is suitable in the PR description and release notes, but not in code comments (maybe in general terms, in one or two places).

rwgk · 2026-06-29T03:38:50Z

+
+
+def _resolve_in_trusted_dirs(normalized_name: str, dirs: list[str]) -> str | None:
+    """Resolve ``normalized_name`` against ``dirs`` only, in order.


Resolve ``normalized_name`` against ``dirs`` in order.

("only" will be puzzling later)

rwgk · 2026-06-29T03:39:35Z

+    Unlike ``shutil.which``, this never consults the current working directory
+    or the ambient ``PATH``. On Windows ``shutil.which`` prepends the process
+    CWD to the search even when an explicit ``path=`` is supplied, which lets a
+    binary sitting in an arbitrary CWD shadow the trusted CUDA / Conda / wheel
+    binary that pathfinder is contracted to discover. Searching the trusted
+    directories explicitly keeps the lookup deterministic and bounded.


See above: please delete this entire paragraph.

rwgk · 2026-06-29T03:43:26Z

+        if not directory or directory in seen:
+            continue


Skipping over empty strings can easily mask bugs (i.e. string slicing or split bug). None doesn't make sense here, too.

A very easy way to be stricter/safer:

if directory in seen: continue assert directory

rwgk · 2026-06-29T03:47:02Z

+             and search that root's bin layout. This finds the utility for users
+             who follow the CUDA install guide and set ``LD_LIBRARY_PATH`` for
+             libraries without also setting ``CUDA_HOME`` / ``CUDA_PATH``.


I think it's better to delete the last sentence: LD_LIBRARY_PATH is mentioned in the previous sentence already, and the second part will just make people wonder unnecessarily/distractingly ("why would one do that?").

rwgk · 2026-06-29T03:47:35Z

+        CTK root listed above; the process working directory and the ambient
+        ``PATH`` are never consulted.


See above: delete part after semicolon.

rwgk · 2026-06-29T03:48:20Z

+
+    # 4. CTK-root canary fallback: only when the explicit trusted dirs above
+    #    miss. Resolve cudart via the dynamic loader (honors LD_LIBRARY_PATH),
+    #    derive the toolkit root, and search its bin layout. PATH is never used.


See above: delete the last line here.

rwgk · 2026-06-29T03:52:20Z

+* :func:`find_nvidia_binary_utility` now resolves binaries through a bounded,
+  deterministic search of trusted directories instead of ``shutil.which``. The
+  process working directory and the ambient ``PATH`` are never consulted, which
+  closes a lookup ambiguity on Windows where ``shutil.which`` prepends the CWD
+  even when an explicit search path is supplied.
+  (`PR #2196 <https://github.com/NVIDIA/cuda-python/pull/2196>`_)
+
+* :func:`find_nvidia_binary_utility` gains a CTK-root canary fallback. When the
+  NVIDIA wheel, ``CONDA_PREFIX``, and ``CUDA_HOME`` / ``CUDA_PATH`` directories
+  all miss, ``cudart`` is resolved through the OS dynamic loader, which honors
+  ``LD_LIBRARY_PATH`` on Linux and the native DLL search on Windows. The CUDA
+  Toolkit root is derived from that path and its ``bin`` layout is searched.
+  This locates the utility for users who follow the CUDA installation guide and
+  set ``LD_LIBRARY_PATH`` for libraries without also setting ``CUDA_HOME`` /
+  ``CUDA_PATH``, while still never falling back to ``PATH``.
+  (`PR #2196 <https://github.com/NVIDIA/cuda-python/pull/2196>`_)


Could you please combine this into one bullet point? I think it's best to also delete the "This locates the utility for users who follow..." sentence here.

aryanputta · 2026-06-29T06:35:01Z

@rwgk I pushed 59254f77cd with the review cleanup. Changes: moved the shared CTK-root canary anchor to _utils/ctk_root_canary.py, removed private-helper prose / PR-local PATH wording from code, made _resolve_in_trusted_dirs() assert on empty directory inputs after duplicate filtering, updated the resolver test, and combined the 1.6.0 release note into one bullet. Local validation: ruff-check, ruff-format, mypy-pathfinder, py_compile, and git diff --check passed. GitHub visible checks are green. Could you have another look when ready?

aryanputta · 2026-06-30T21:35:31Z

@rwgk gentle nudge on this one. The CTK-root canary fallback is folded in (c3d39f0), pre-commit.ci is green, and the only red check is the metadata/assignee gate I can't set from a fork. Happy to adjust the canary ordering or the minor-version bump if you'd like it shaped differently. Let me know when you get a chance to take another look.

github-actions Bot added the cuda.pathfinder Everything related to the cuda.pathfinder module label Jun 10, 2026

rwgk added this to the cuda.pathfinder next milestone Jun 10, 2026

rwgk assigned aryanputta Jun 10, 2026

rwgk added the P1 Medium priority - Should do label Jun 10, 2026

leofang assigned rwgk Jun 11, 2026

rwgk mentioned this pull request Jun 16, 2026

pathfinder: add sm_arch bitcode lookup #2231

Closed

rwgk reviewed Jun 29, 2026

View reviewed changes

fix(pathfinder): address binary finder review

59254f7

		On Windows the CTK ships binaries under ``bin/x64`` (CTK 13), ``bin/x86_64``,
		and ``bin`` (CTK 12); on Linux they live in ``bin``.



		def _resolve_in_trusted_dirs(normalized_name: str, dirs: list[str]) -> str \| None:
		"""Resolve ``normalized_name`` against ``dirs`` only, in order.

		CTK root listed above; the process working directory and the ambient
		``PATH`` are never consulted.

Uh oh!

Conversation

aryanputta commented Jun 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Fix

Tests

Uh oh!

copy-pr-bot Bot commented Jun 10, 2026

Uh oh!

rwgk commented Jun 10, 2026

Uh oh!

aryanputta commented Jun 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

aryanputta commented Jun 23, 2026

Uh oh!

rwgk left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

aryanputta commented Jun 29, 2026

Uh oh!

aryanputta commented Jun 30, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

aryanputta commented Jun 10, 2026 •

edited

Loading

aryanputta commented Jun 11, 2026 •

edited

Loading