fix: add lint rule to detect bare pip install and fix beval.yml violation - #2548
Conversation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #2548 +/- ##
==========================================
+ Coverage 82.53% 86.95% +4.42%
==========================================
Files 155 93 -62
Lines 21123 12430 -8693
Branches 13 0 -13
==========================================
- Hits 17434 10809 -6625
+ Misses 3687 1621 -2066
+ Partials 2 0 -2
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
Chris Montazer (rezatnoMsirhC)
left a comment
Chris Montazer (rezatnoMsirhC)
left a comment
There was a problem hiding this comment.
Thanks for the contribution. One item not covered by the diff: .github/skills/experimental/powerpoint/SKILL.md has pip install pillow in a dependency note (~line 455). There is no reason for bare pip here — please update to uv pip install pillow. This is exactly the kind of documentation drift the new lint rule is intended to prevent, and it is also an argument for the ignore-comment mechanism suggested on the lint script itself: with broader scan coverage and explicit <!-- pip-install-ok --> markers for genuine bootstrap exceptions, this would have been caught automatically.
| echo "${{ github.workspace }}/evals/beval/node_modules/.bin" >> "$GITHUB_PATH" | ||
|
|
||
| - name: Install uv | ||
| uses: astral-sh/setup-uv@c771a70e6277c0a99b617c7a806ffedaca235ff9 # v9.0.0 |
There was a problem hiding this comment.
This is a pre-existing pattern in the repository: astral-sh/setup-uv at this same commit SHA is already used in eval-validation.yml, fuzz-tests.yml, and pytest-tests.yml. This PR adds one more usage of an already-accepted pattern.
The action is pinned to a full commit SHA (c771a70e6277c0a99b617c7a806ffedaca235ff9 # v9.0.0), which is the repo's mitigating control for unverified actions. To fully resolve the underlying concern, the repo could replace the action with a manual install step (curl -LsSf https://astral.sh/uv/install.sh | sh) or add astral-sh/setup-uv to a poutine allowlist. Either is a repo-level decision beyond the scope of this PR.
| contents: read | ||
|
|
||
| jobs: | ||
| check-bare-pip-install: |
| with: | ||
| soft-fail: false | ||
|
|
||
| pip-install-lint: |
93cc779 to
71d99d6
Compare
| continue | ||
| } | ||
|
|
||
| if ($line -match "\bpip3?\s+install\b" -and $line -notmatch "\buv\s+pip3?\s+install\b" -and $line -notmatch "%pip\s+install") { |
There was a problem hiding this comment.
The allowlist checks exempt the entire line when any uv pip install or %pip install substring exists. For example, uv pip install allowed && pip install bypassed passes this required check; I reproduced exit code 0 at the current head. Please evaluate each pip install occurrence independently, or remove approved occurrences before checking for any bare match, and add a regression test for mixed commands on one line.
There was a problem hiding this comment.
The scanner misses several maintained repository surfaces: .sh is absent from the extension allowlist, the segment-wide evals exclusion also suppresses scripts/evals, and the default traversal misses hidden .github content outside workflows. I reproduced bare installs in all three surfaces passing with exit code 0. Please enumerate tracked files deterministically, include executable shell formats, narrow the eval exclusion to the intended corpus, and add regression tests for these paths.
| - name: Checkout code | ||
| uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 |
There was a problem hiding this comment.
This job executes a PR-controlled script immediately after checkout, but checkout credentials are persisted by default. Disable credential persistence so modified PR code cannot recover the read token and to follow the repository's workflow credential convention.
| - name: Checkout code | |
| uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 | |
| - name: Checkout code | |
| uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 | |
| with: | |
| persist-credentials: false |
| $testDir = "$PSScriptRoot/TestLintDir" | ||
| if (Test-Path $testDir) { Remove-Item -Recurse -Force $testDir } | ||
| New-Item -ItemType Directory -Path $testDir | Out-Null | ||
| } | ||
|
|
||
| AfterAll { | ||
| if (Test-Path $testDir) { Remove-Item -Recurse -Force $testDir } | ||
| } | ||
|
|
||
| AfterEach { | ||
| if (Test-Path $testDir) { Remove-Item -Recurse -Force $testDir } | ||
| New-Item -ItemType Directory -Path $testDir | Out-Null |
There was a problem hiding this comment.
Nit: These fixtures are created inside the repository checkout, so parallel or interrupted test runs can collide or leave workspace state. Please use Pester's $TestDrive for test-owned files.
| $testDir = "$PSScriptRoot/TestLintDir" | |
| if (Test-Path $testDir) { Remove-Item -Recurse -Force $testDir } | |
| New-Item -ItemType Directory -Path $testDir | Out-Null | |
| } | |
| AfterAll { | |
| if (Test-Path $testDir) { Remove-Item -Recurse -Force $testDir } | |
| } | |
| AfterEach { | |
| if (Test-Path $testDir) { Remove-Item -Recurse -Force $testDir } | |
| New-Item -ItemType Directory -Path $testDir | Out-Null | |
| $testDir = Join-Path $TestDrive "TestLintDir" | |
| New-Item -ItemType Directory -Path $testDir | Out-Null | |
| } | |
| AfterEach { | |
| if (Test-Path $testDir) { Remove-Item -Recurse -Force $testDir } | |
| New-Item -ItemType Directory -Path $testDir | Out-Null |
There was a problem hiding this comment.
Current main intentionally deleted this workflow in PR #2552 when beval was retired in favor of Vally agent-conformance suites. Please merge current main, accept this file's deletion, and remove the obsolete beval hunk from this PR. The replacement workflow does not install beval, so this install change does not need to be migrated.
| # Copyright (c) 2026 Microsoft Corporation. All rights reserved. | ||
| # SPDX-License-Identifier: MIT | ||
|
|
||
| Describe "Invoke-PipInstallLint.ps1" { |
There was a problem hiding this comment.
Nit: Add the standard Pester module requirement and Unit tag so this suite follows the repository's discovery and filtering conventions.
| # Copyright (c) 2026 Microsoft Corporation. All rights reserved. | |
| # SPDX-License-Identifier: MIT | |
| Describe "Invoke-PipInstallLint.ps1" { | |
| #Requires -Modules Pester | |
| # Copyright (c) 2026 Microsoft Corporation. All rights reserved. | |
| # SPDX-License-Identifier: MIT | |
| Describe "Invoke-PipInstallLint.ps1" -Tag "Unit" { |
5 participants
Description
Adds CI enforcement for the repo's uv-first Python convention by introducing a reusable lint workflow that detects bare
pip installcalls in workflows, scripts, and Python files. Fixes the one known existing violation inbeval.ymland updates experimental skill error strings to recommenduv pip install.Companion to #1313, which adds the corresponding prohibitions to the Python instruction files. Without this enforcement backstop, those rules have no CI validation and regressions can silently appear.
Related Issue(s)
Fixes #2500
Companion to #1313
Type of Change
Select all that apply:
Code & Documentation:
Infrastructure & Configuration:
AI Artifacts:
hve-builderand addressed all actionable findings.github/instructions/*.instructions.md).github/prompts/*.prompt.md).github/agents/*.agent.md).github/skills/*/SKILL.md).github/hooks/*/*.json)evals/)Other:
.ps1,.sh,.py)Testing
All 4 local test scenarios passed successfully:
python scripts/lint_pip_install.py→Success: No bare 'pip install' calls found.run: pip install malicious-packageinto a fake workflow → correctly flagged and exited 1.pip install mock-packageintoevals/fake_eval_test.py→ correctly ignored per acceptance criteria.run: uv pip install fastapiinto a fake workflow → correctly passed.Additionally verified that
.venvdirectories, YAMLname:metadata lines, andTHIRD-PARTY-NOTICESare properly excluded to prevent false positives.Checklist
Required Checks
AI Artifact Contributions
hve-builderreview mode to review contributionhve-builderreviewRequired Local Checks
npm run validate:local(N/A — no docs/PS/md changed)npm run validate:docs(N/A)npm run spell-check(N/A — no prose changed)npm run lint:md-links(N/A — no URLs added)Security Considerations
All GitHub Actions in new and modified workflows are SHA-pinned per repo convention:
actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83ddactions/setup-python@5fda3b95a4ea91299a34e894583c3862153e4b97astral-sh/setup-uv@c771a70e6277c0a99b617c7a806ffedaca235ff9The
beval.ymlinstall remains pinned to commit SHAd9f46c24f03b0b806d928a8a8ce2fc66d8e470fbto mitigate supply-chain risk.Additional Notes
Files changed (7 total):
scripts/lint_pip_install.py.venv/evals/THIRD-PARTY-NOTICESexclusions.github/workflows/pip-install-lint.yml.github/workflows/pr-validation.ymlpip-install-lintjob calling the reusable workflow.github/workflows/beval.ymlpip installwithuv pip install --system, addedsetup-uvstep.github/skills/experimental/powerpoint/scripts/export_slides.pyuv pip install.github/skills/experimental/powerpoint/scripts/export_svg.pyuv pip install.github/skills/experimental/powerpoint/scripts/render_pdf_images.pyuv pip install