Resolve the CLI and helper programs via a safe PATH lookup (never the working directory) - #1188
Draft
seanyeoh-ant wants to merge 7 commits into
Draft
Conversation
Add claude_agent_sdk._internal.executable: find_executable / require_executable / resolve_argv / run and ExecutableNotFoundError. find_executable is a hand-written PATH walk that searches only the fully-absolute entries of PATH -- never the current working directory, neither implicitly (as shutil.which and CreateProcess do on Windows) nor via "."/empty/relative entries -- and on Windows only ever returns a native .exe/.com image, never a .bat/.cmd shim or an extensionless file. A name with a directory part is the caller's explicit choice and is only made absolute. Whatever it returns is absolute and normalised, ready to be handed to the OS as argv[0]. The platform rules are pure ntpath/posixpath string functions, so the Windows behaviour is unit-tested on POSIX CI as well. The module docstring states the invariant and the named guarantees (G1-G5, D1) shared with the other Anthropic SDKs; tests/test_executable.py implements the shared vectors (V1-V12, W1-W3, P1). Motivated by HackerOne #3888880 (CWE-427): the callsites are migrated in the following commit.
HackerOne #3888880 (CWE-427): on Windows installs without the bundled
CLI, _find_cli() returned shutil.which("claude"), and CPython's
shutil.which searches the current directory first on win32 -- so a
claude.exe planted in whatever directory the application runs from (a
cloned repository, an extracted archive) was spawned, twice, with the
developer's environment. The same class applied to the bare "git" the
session listing runs and, in principle, to a bare options.cli_path.
- transport: _find_cli() resolves "claude" with find_executable --
absolute PATH entries only, never the current directory, and on
Windows only a native claude.exe/claude.com. That subsumes the old
which("claude") / which("claude.exe") / _is_windows_native_exe
preference dance (a .cmd shim or extensionless wrapper in an earlier
PATH directory can no longer shadow a native exe in a later one, since
neither is ever a candidate). A shim-only Windows machine still gets
the explanatory batch-script refusal: the shim is detected with an
explicit (".cmd", ".bat") allow-list and handed to
_reject_windows_batch_cli, never spawned.
- transport: connect() settles one absolute CLI path up front
(_settle_cli_path) for both the version probe and the main spawn. An
explicit cli_path with a directory part is only made absolute (the
caller's decision, checked against the batch-script rule as spelled);
a bare cli_path="claude" now goes through find_executable instead of
being handed to the OS to search.
- sessions: `git worktree list` runs through executable.run(), which
resolves git safely and raises an OSError subclass when there is none,
so the existing "git unavailable -> []" path is unchanged.
- session_resume: the macOS Keychain read invokes /usr/bin/security by
absolute path (a SIP-protected fixed location) instead of "security".
Tests: the Windows discovery tests now stub find_executable instead of
shutil.which (the two that exercised CPython's PATHEXT quirks --
"claude.exe.cmd" and the extensionless which() hit -- are replaced by
find_executable's own W2/W3 vectors); new regression tests plant
claude / claude.exe / claude.cmd / git in the working directory with
".", "" and relative entries on PATH and assert against the real
filesystem that nothing planted is ever discovered or run, that a bare
cli_path is looked up safely, and that a relative cli_path is settled
to a single absolute path.
G5 -- one implementation, enforced: - pyproject.toml: enable ruff TID251 and ban shutil.which and distutils.spawn.find_executable with a message pointing at claude_agent_sdk._internal.executable. Only scripts/ (build tooling that runs before the package is importable) is exempt. - tests/test_executable_invariant.py: parses every module under src/ and fails if a subprocess / anyio / asyncio / os.exec* / os.spawn* / os.system-style call names its program with a bare or relative string literal (or a module-level string constant), or if an ambient lookup API is imported or referenced. Parametrized bad/good snippets prove the scanner is not vacuous; run against origin/main it flags exactly the four sites the previous commit migrated.
…import
D1 -- defense in depth, deliberately in its own commit; it is NOT the
fix for HackerOne #3888880 (the previous commits are: the SDK no longer
asks the OS to search for any program). Importing the transport module
on Windows now does
os.environ.setdefault("NoDefaultCurrentDirectoryInExePath", "1"),
mirroring the TypeScript Agent SDK, which sets the same variable
process-wide at import. With it present, CreateProcess -- and
shutil.which on Python >= 3.12 -- stop searching the current directory
for bare command names, which also covers the CLI's own child processes
since they inherit it. Python < 3.12's shutil.which ignores it, which is
why it cannot be the fix. A value the application set itself is left
alone.
Add a "Spawning External Programs" section to the README's Development section and to CLAUDE.md: use _internal/executable.py (find_executable / require_executable / run) instead of bare program names or shutil.which, why (CWE-427: the working directory is searched on Windows), and how CI enforces it (ruff TID251 banned-api + tests/test_executable_invariant.py). CHANGELOG.md is intentionally untouched: this repo generates release entries at publish time (docs: update changelog for vX.Y.Z), and tests/test_changelog.py only admits "## X.Y.Z" headings.
OSError reconstructs as cls(errno, strerror, filename) when unpickled; give the one-argument subclass a matching __reduce__ so it survives multiprocessing / pytest-xdist boundaries.
|
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1188 +/- ##
=======================================
Coverage ? 90.70%
=======================================
Files ? 24
Lines ? 4293
Branches ? 0
=======================================
Hits ? 3894
Misses ? 399
Partials ? 0 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
…ropic-sdk-python
Behaviour parity with the sibling module (anthropic-sdk-python
src/anthropic/lib/_executable.py, "harden the resolver's run/resolve_argv
edges found in review"); the modules are meant to stay behaviourally
identical across SDKs:
- run() resolves argv[0] against the PATH the child will actually see:
the one in an explicit env={...} when it carries a PATH, else this
process's -- matching what subprocess.run itself searches on POSIX,
still absolute entries only, and never falling back to os.defpath.
- resolve_argv() (and therefore run()) rejects a plain string argv with
a clear TypeError instead of "resolving" its first character.
Tests ported with the sibling's names/vectors:
test_run_resolves_against_the_path_the_child_will_see,
test_executable_not_found_error_survives_pickling (split out; adds the
copy.copy and errno/str round-trip checks), and the str-argv assertions
in test_resolve_argv_replaces_only_the_program. D1 stays process-wide in
this repo (no per-child env helper).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What this is
On Windows, when no bundled CLI is present (sdist / source installs, Windows-on-ARM),
_find_cli()returnedshutil.which("claude")— and CPython'sshutil.whichsearches the current working directory first on Windows. So aclaude.exedropped into whatever directory the application runs from (a cloned repository, an extracted archive) was returned as.\claude.EXEand spawned twice (version probe + main process) with the developer's environment (HackerOne #3888880, CWE-427). The Claude Code CLI already solved this class for itself with a safe resolver plus a lint ban plusNoDefaultCurrentDirectoryInExePath; this PR brings the same guarantees to this SDK through one small internal module that every program launch now goes through, and enforces it in CI so it cannot quietly come back.Code changes
src/claude_agent_sdk/_internal/executable.py(find_executable/require_executable/resolve_argv/run,ExecutableNotFoundError). A hand-writtenPATHwalk: only fully-absolutePATHentries are searched — never the current directory, whether implicitly (asshutil.whichandCreateProcessdo on Windows) or through./ empty / relative entries — and on Windows only a nativeclaude.exe/claude.comis ever returned, never a.bat/.cmdshim or an extensionless file. A name with a directory part is the caller's explicit choice and is only made absolute. The Windows rules are purentpathstring functions, so they are unit-tested on the Linux/macOS CI legs too. (shutil.which(name, path=...)cannot be made safe by sanitisingpath: CPython still prepends the current directory on Windows and still appends.BAT/.CMDfromPATHEXT.)_find_cli()usesfind_executable("claude")instead ofshutil.which("claude")/shutil.which("claude.exe"). Prevents: aclaude.exeplanted in the directory you run your script from being picked over (or instead of) the installed CLI. Because a.cmdshim or extensionless wrapper is never a candidate any more, the old "prefer a native exe over a shadowing shim" dance (_is_windows_native_exe) goes away: the native executable is found wherever it sits onPATH. A shim-only Windows machine still gets the existing explanatory "refusing to execute batch script" error — the shim is detected with an explicit(".cmd", ".bat")allow-list and handed to_reject_windows_batch_cli, never spawned. A machine with only an extensionless git-bash/WSL wrapper now gets the "Claude Code not found, install the native claude.exe" message instead of an opaque WinError 193 at spawn.connect()settles one absolute CLI path up front (_settle_cli_path) and both spawns use exactly that. An explicitcli_pathwith a directory part is checked against the batch-script rule as spelled, then made absolute against the process's working directory (nothing is searched; a missing file still raisesCLINotFoundErroras before). Note: previously, on POSIX only, the main spawn resolved a relativecli_pathagainstoptions.cwdwhile the version probe resolved it against the process's working directory; both now use the process's working directory, which is what Windows already did. A barecli_path="claude"used to be handed to the OS to look up (current directory first on Windows); it now goes throughfind_executable._internal/sessions.py:git worktree listruns throughexecutable.run([...]). Prevents: agit/git.exesitting in the project directory being listed (or reachable through a./ emptyPATHentry) being run instead of the installed git.ExecutableNotFoundErroris anOSError, so "git unavailable →[]" behaves exactly as before._internal/session_resume.py: the macOS Keychain read calls/usr/bin/securityby absolute path instead of"security". It is a SIP-protected system binary at a fixed location on every supported macOS, so an absolute constant is the simplest way to never look anything up.os.environ.setdefault("NoDefaultCurrentDirectoryInExePath", "1")on Windows, mirroring the TypeScript Agent SDK which sets the same variable process-wide at import. That makesCreateProcess(andshutil.whichon Python ≥ 3.12) skip the current directory for anything downstream that still does an OS-level bare-name lookup, the CLI's own children included. Python < 3.12 ignores it, which is why the real fix is the resolver above. Easy to drop independently if unwanted.Guarantees
Named in the module docstring so tests, docs and the sibling SDKs can cross-reference them:
subprocess/anyio.., empty, relative, drive-relative (C:bin) or rooted-but-driveless (\bin)PATHentries; unset/emptyPATHfinds nothing..exe/.comby default) are candidates; never.bat/.cmd(the BatBadBut / CVE-2024-27980 class fixed in Refuse to spawn .bat/.cmd CLI scripts on Windows to prevent command injection #1127), never extensionless files. The allow-list is a parameter so a shim can be detected without being runnable.shutil.which/distutils.spawn.find_executable(TID251banned-api,scripts/build tooling exempt), andtests/test_executable_invariant.pyparsessrc/and fails on any spawn call whose program is a bare or relative literal.NoDefaultCurrentDirectoryInExePathset at transport import on Windows (defense in depth only).Tests
tests/test_executable.py— the shared cross-SDK vectors against real temp files withmonkeypatch.chdir/PATH: absolute entry found (V1); empty,., relative entries skipped even though the plant is right there (V2–V5); non-executable file and directory-named-like-the-tool skipped (V6, V7);./tooland absolute paths honoured as explicit choices (V8, V9); result absolute + normalised (V10); degenerate names and unset/emptyPATHfind nothing (V11, V12); Windows entry/candidate rules and an end-to-end Windows walk under a fake filesystem, including "npm shim in an earlierPATHdirectory does not shadow the native exe in a later one" (W1–W3); POSIX backslash-is-not-a-separator (P1); plusrequire_executable/resolve_argv/run(including thatrunnever spawns a plant, refusesshell=/executable=and a plain-string argv, resolvesargv[0]against thePATHinside an explicitenv=the waysubprocess.runwould — absolute entries only, neveros.defpath— and thatExecutableNotFoundErrorsurvives pickling). These edge behaviours and test names match the sibling module in anthropic-sdk-python so the suites stay comparable. Vectors are written to pass on the Windows CI leg as well (tool.exealongsidetool).find_executablenot mocked:tests/test_transport.py::TestCLIDiscoveryNeverUsesTheWorkingDirectoryplantsclaude/claude.exe/claude.cmdin the working directory with"",.and a relative entry onPATHand asserts discovery raisesCLINotFoundError,connect()spawns nothing (and the POSIX plant's marker file never appears), a real install later onPATHis found straight past the plant, a barecli_path="claude"is looked up safely, and a relativecli_pathis settled to one absolute path for both spawns.tests/test_sessions.pydoes the same for a plantedgit;tests/test_session_resume.pyasserts the Keychain read invokes/usr/bin/security.find_executableinstead ofshutil.which; the two that exercised CPythonwhich()quirks that no longer apply (claude.exe.cmdfromPATHEXT, the extensionlesswhich()hit) are covered by the module's own W2/W3 vectors instead.tests/test_executable_invariant.py— the G5 scan, with parametrized bad/good snippets proving it is not vacuous; pointed atorigin/mainit flags exactly the four call sites this PR migrates.ruff check src/ tests/ scripts/,ruff format --check src/ tests/ scripts/,mypy src/ scripts/clean;python -m pytest tests/→ 1447 passed, 6 skipped.Docs
_internal/executable.py: the invariant paragraph, G1–G5/D1, and a keep-in-sync note naming the sibling SDKs.CLAUDE.md: a short "Spawning External Programs" rule for contributors (what to use, why, how CI enforces it).CHANGELOG.mddeliberately untouched: release entries here are generated at publish time, andtests/test_changelog.pyonly admits## X.Y.Zheadings.Not in this PR
ANTHROPIC_*/ other environment variables from helper child processes — a separate concern from which binary runs.scripts/download_cli.pystill usesshutil.which("claude"): it is build tooling that runs on CI runners / maintainer machines before the package is importable, so it is exempted from the lint ban rather than routed through the SDK module.