Route Python packages through Azure Artifacts#8576
Conversation
|
/azp run |
|
Azure Pipelines: There may be pipelines that require an authorized user to comment /azp run to run. |
|
Azure Pipelines: 1 pipeline(s) were filtered out due to trigger conditions. |
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
| cache_path = os.path.join(cache_dir, filename) | ||
| except Exception: | ||
| cache_path = None | ||
| os.makedirs(cache_dir, exist_ok=True) |
|
/azp run |
|
Azure Pipelines: 1 pipeline(s) were filtered out due to trigger conditions. |
PipAuthenticate@1 on 1ES hosted agents emits PIP_INDEX_URL using the legacy `<org>.pkgs.visualstudio.com` host, while local runs and other agents use the canonical `pkgs.dev.azure.com` host. Both are the same Azure Artifacts service under different DNS names and both are already in the redirect denylist for public PyPI, so accept either as a valid index host and validate every subsequent request against the same rule. Verified end-to-end against both hosts and confirmed a bogus host is still rejected. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
/azp run |
|
Azure Pipelines: 1 pipeline(s) were filtered out due to trigger conditions. |
|
/azp run |
|
Azure Pipelines: 1 pipeline(s) were filtered out due to trigger conditions. |
|
/azp run |
|
Azure Pipelines: 1 pipeline(s) were filtered out due to trigger conditions. |
StellaHuang95
changed the title
|
Azure Pipelines: There may be pipelines that require an authorized user to comment /azp run to run. |
1 similar comment
|
Azure Pipelines: There may be pipelines that require an authorized user to comment /azp run to run. |
|
🔒 Automated review in progress — @rchiodo is auto-reviewing this PR. |
|
🔒 Automated review in progress — @heejaechang is auto-reviewing this PR. |
|
/azp run |
|
Azure Pipelines: Successfully started running 1 pipeline(s). |
|
| f.write(data) | ||
|
|
||
| fragment = urllib.parse.parse_qs(urllib.parse.urlsplit(url).fragment) | ||
| expected_hashes = fragment.get("sha256") |
There was a problem hiding this comment.
The optional wheel cache lost all of its error handling. Previously os.makedirs, the cache read, and the cache write were each wrapped in try/except, so a broken or opt-in PTVS_PYPI_WHEEL_CACHE_DIR (read-only mount, permission error, full disk, concurrent job) could never fail the install. Now any of those conditions will hard-fail the entire build, turning an optional accelerator into a load-bearing dependency. Restore best-effort semantics (degrade to a direct download on cache failure), or explicitly document that the cache is mandatory-when-configured.
| with open(cache_path, "wb") as f: | ||
| f.write(data) | ||
|
|
||
| fragment = urllib.parse.parse_qs(urllib.parse.urlsplit(url).fragment) |
There was a problem hiding this comment.
The cache write is non-atomic and happens before SHA-256 verification. A truncated/corrupt download gets persisted to cache_path before the hash check runs, so every subsequent run reads the poisoned file, re-verifies, and fails again — a persistent build break until the cache is manually purged. With the shared-cache parallelism this feed change enables, a partial f.write() can also be read by another job. Verify the SHA-256 on the downloaded bytes before writing, and write via a temp file + os.replace atomic rename.
| wheel_name, version, _, _ = parse_wheel_filename(filename) | ||
| if canonicalize_name(wheel_name) != expected_name: | ||
| continue | ||
|
|
There was a problem hiding this comment.
parse_wheel_filename can raise InvalidWheelFilename (or InvalidVersion on a non-PEP440 version), and neither is caught in get_package_data. A single malformed .whl-suffixed link on the feed's simple-index page for the requested package would abort all builds — a hard-failure the old JSON path didn't have. Wrap the call in try/except (InvalidWheelFilename, InvalidVersion): continue to skip unparseable links.
|
|
||
|
|
||
| class _SfiRedirectHandler(url_lib.HTTPRedirectHandler): | ||
| # Azure Artifacts responds with a 303 pointing at a SAS-signed blob URL; |
There was a problem hiding this comment.
The _SfiRedirectHandler comment claims it refuses "any redirect that leaves Microsoft-owned infrastructure," but the implementation only raises for the 5-host FORBIDDEN_HOSTS deny-list — a redirect to any other non-forbidden host would still be followed. (The Basic credential is correctly not resent, so no secret leaks.) Update the comment to describe the actual deny-list behavior so a future maintainer doesn't 'tighten' it based on the misleading description and break legitimate *.blob.core.windows.net redirects.
|
Solid, security-focused change that meets the SFI goal. A few robustness edge cases around the optional wheel cache and feed-link parsing are worth tightening before merge, but none are blocking given the passing pipeline validation. |
rchiodo
left a comment
rchiodo
left a comment
There was a problem hiding this comment.
Approved via Review Center.
4 participants
Summary
Route all pipeline Python package downloads through the team-owned
DevDiv/Pylance_PublicPackagesAzure Artifacts feed to resolve the SFI-ES4.2.4 public PyPI violation.PipAuthenticate@1pypi.orgJSON andfiles.pythonhosted.orgrequests with the authenticated PEP 503 feedValidation
Pylance_PublicPackages