[azure-ai-ml] Validate artifact tool download request URLs - #49186
Open
ayushhgarg-work wants to merge 5 commits into
Open
ayushhgarg-work wants to merge 5 commits into
ayushhgarg-work wants to merge 5 commits into
Conversation
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
ayushhgarg-work
requested review from
Justin Firsching (JustinFirsching),
Klein Hu (NonStatic2014),
Amit Chauhan (achauhan-scc),
Arun (arunsu),
Jayesh Tanna (jayesh-tanna),
kingernupur,
Nikolay Rovinskiy (nick863),
Ayush Mishra (novaturient95),
Razvan Tanase (rtanase),
Riti Sharma (sharma-riti) and
Vivek Ramaswamy (vivram)
as code owners
September 25, 2026 05:20
|
Azure Pipelines: Successfully started running 1 pipeline(s). 10 pipeline(s) were filtered out due to trigger conditions. There may be pipelines that require an authorized user to comment /azp run to run. |
Contributor
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
A valid archive without the ArtifactTool executable is incorrectly marked as installed.
Get a fresh assessment by requesting another Copilot review.
Review effort: Balanced
Findings: 1
Open (1)
What changed in this PR
Hardens Azure DevOps ArtifactTool downloads against unsafe URLs, redirects, credential forwarding, and failed extraction.
Changes:
- Adds strict URL validation and redirect handling.
- Cleans failed installations and narrows fallback exceptions.
- Adds security-focused unit tests and fixtures.
| File | Description |
|---|---|
azure/ai/ml/_utils/_artifact_utils.py |
Hardens ArtifactTool download and installation. |
tests/utils/unittests/test_artifact_tool_requests.py |
Tests validation and failure scenarios. |
tests/test_utilities/artifact_fixtures.py |
Adds isolated ArtifactTool fixtures. |
tests/test_utilities/utils.py |
Isolates artifact-cache test state. |
CHANGELOG.md |
Documents the fix. |
💡 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.
Amit Chauhan (achauhan-scc)
approved these changes
Sep 25, 2026
6 tasks
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
Linux installations omit executable permissions, leaving the downloaded artifact tool unusable.
Get a fresh assessment by requesting another Copilot review.
Review effort: Balanced
Findings: 1
Resolved since last review (1)
Comment on lines
+273
to
276
| tool_name = "artifacttool.exe" if os_name == "Windows" else "artifacttool" | ||
| if not (artifacts_tool_path / tool_name).is_file(): | ||
| raise RuntimeError(f"Artifact tool archive does not contain {tool_name}.") | ||
| os.environ["AZURE_DEVOPS_EXT_ARTIFACTTOOL_OVERRIDE_PATH"] = str(artifacts_tool_path.resolve()) |
ayushhgarg-work
enabled auto-merge (squash)
September 25, 2026 08:01
This branch has not been deployed
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.


Description
Validate request destinations in the Azure DevOps artifact-tool download fallback used by component
additional_includes.Cache containment and concurrent cache handling are addressed separately in #49185. This PR preserves the existing cache implementation and authentication scope; it does not add a speculative CDN allowlist or release/version changes. The two branches were checked for conflict-free integration.
Validation
Review follow-up at
cd6710979e638092a0ed05ab1f448398c3bf3846:The original request fix was also checked on Windows and native Linux, including both override modes. The final SDK review branches merge without conflicts in either order; their combined tree passed all 400 selected tests through the official MCP tool in both override modes.
Source imports were asserted against the selected checkout and playback used the owned loopback proxy. These local results are not a claim that the latest PR CI is green.
All SDK Contribution checklist:
General Guidelines and Best Practices
Testing Guidelines