Repository navigation
fix(detectors): a rooted search path is refused on 3.13 too - #170
Merged
Merged
Conversation
A detector's `search_paths` names directories under the project its tool is looked for in, and `_escapes_the_project` refuses an entry naming anything else. It asked `ntpath.isabs` for the Windows-rooted form — and CPython 3.13 stopped counting a single leading (back)slash as rooted, so `\tools` walked past a guard written to stop exactly it. Nothing else in the expression covered the case. On Windows the consequence is the one the guard exists to prevent: `project_dir / "\tools"` is `C:\tools`, so `tool_executable` searches, and can spawn, a binary from outside the project. On POSIX the entry names a directory under the project that will not exist, and is harmless. The separators are normalised on the next line anyway, so the leading one is read off there instead of asked of an interpreter whose answer moved. Once `splitdrive` has returned empty, an empty first component is what the older `ntpath.isabs` meant, and it subsumes `posixpath.isabs` — which is now unused and dropped. Swept against the pre-3.13 oracle over both reviewers' generated inputs, the two expressions differ on the empty string alone, which `_names_a_directory` refuses before this function is reached. `test_a_rooted_search_path_is_refused` already pinned this and needed no change: it is red on 3.13 without the fix and green with it. CI never saw that, because `.github/workflows/ci.yml` pins 3.11 on both legs while the classifiers promise 3.13 — left alone here, as the matrix is the maintainer's call, and raised on the issue. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Collaborator
|
Thanks for the fix. Few remarks there were not merge stoppers, but since you are contributing more I'd like you to be aware:
|
Contributor
Author
|
Haha, the docstring in the PR is already the 30% shortened result of my pushback. Good to know that we're on the same side ;) Will push back more aggressive in future 👍 |
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
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 cann
2E8F
ot 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.
Closes #166.
_escapes_the_projectusedntpath.isabsto decide if asearch_pathsentry was rooted. Python 3.13 changed that function: a path that starts with one single (back)slash is no longer rooted. So\toolswas no longer refused.On Windows this has a real effect.
project_dir / "\tools"isC:\tools, so a tool outside the project could be found and started. On POSIX the entry is harmless.The fix no longer calls
isabs. The next line already splits the path into components. If the first component is empty, the path starts with a separator, so it is rooted.posixpathis now unused, so the import is removed.test_a_rooted_search_path_is_refusedalready covers this case: it fails on 3.13 without the fix and passes with it. I also ran the full suite on 3.11 and on 3.13, with the same result on both. CI cannot show you this, because it runs 3.11 only.Two things outside this PR:
ci.yml:25), so it did not catch this.pyproject.tomlsays 3.13 is supported. The options for the matrix and their cost are in On Python 3.13 a plugin'ssearch_pathscan point outside the project, and CI never runs 3.13 #166. I can open a separate issue for it.\\server\share) have no test. This existed before my change and is not affected by it: they are refused by thesplitdriveline, which is unchanged. I can open an issue for that too.