Repository navigation
Fix git.py logic - #1524
Open
triptijain2112 wants to merge 15 commits into
Open
Fix git.py logic#1524triptijain2112 wants to merge 15 commits into
triptijain2112 wants to merge 15 commits into
Conversation
triptijain2112
requested review from
UnyieldingAvocado,
Bhavesh Bhati (bhaveshbhati) and
mishrarish
as code owners
October 6, 2026 04:41
Removed unnecessary blank line in the _get_git function.
Refactor git executable retrieval to use a new function and improve path handling.
Refactor git executable retrieval to use caching and remove global variable.
triptijain2112
changed the title
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
Repository-boundary detection is incomplete and the mandatory security regression tests are missing.
Review effort: Balanced
Findings: 1
Open (1)
What changed in this PR
Hardens Git executable discovery to avoid repository-root executable planting.
Changes:
- Adds canonical PATH-based Git resolution.
- Routes Git subprocess calls through the hardened locator.
- Rejects overlapping paths and non-executable candidates.
| File | Description |
|---|---|
azure-devops/azext_devops/dev/common/git.py |
Implements trusted Git executable discovery and updates callers. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
This file contains regression tests for the CWD git-planting vulnerability, ensuring that only trusted git binaries are selected and that planted executables in the current working directory or its subdirectories are rejected.
Updated unit tests for the _paths_overlap helper to use os.path.join for cross-platform compatibility.
Added tests to ensure Git remote discovery does not invoke canary executables and does not leak Azure DevOps credentials to subprocesses.
Added environment sanitization for Git subprocesses to remove Azure DevOps CLI credential/config variables.
triptijain2112
changed the title
Removed unnecessary comments and documentation in git.py.
Remove redundant comments and docstrings from test cases.
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 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.
Please make sure the code is following contribution guidelines in CONTRIBUTING.md