-
Notifications
You must be signed in to change notification settings - Fork 253
fix: add lint rule to detect bare pip install and fix beval.yml violation #2548
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from 4 commits
d1e42c4
febbd5e
f7d0720
0d22044
f96c66c
71d99d6
653da41
d0147ec
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Current |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -41,11 +41,14 @@ | |
| npm ci --prefix evals/beval | ||
| echo "${{ github.workspace }}/evals/beval/node_modules/.bin" >> "$GITHUB_PATH" | ||
|
|
||
| - name: Install uv | ||
| uses: astral-sh/setup-uv@c771a70e6277c0a99b617c7a806ffedaca235ff9 # v9.0.0 | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is a pre-existing pattern in the repository: The action is pinned to a full commit SHA ( |
||
|
|
||
| - name: Install beval | ||
| # beval is hosted under a personal account (vyta) while an org-owned | ||
| # home is evaluated. The install is pinned to a specific commit SHA to | ||
| # mitigate supply-chain risk in the interim. | ||
| run: pip install --no-cache-dir "beval[all] @ git+https://github.com/vyta/beval.git@d9f46c24f03b0b806d928a8a8ce2fc66d8e470fb#subdirectory=python" | ||
| run: uv pip install --system --no-cache-dir "beval[all] @ git+https://github.com/vyta/beval.git@d9f46c24f03b0b806d928a8a8ce2fc66d8e470fb#subdirectory=python" | ||
|
|
||
| - name: Start agent (TCP) | ||
| env: | ||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,18 @@ | ||||||||||||||
| name: Pip Install Lint | ||||||||||||||
|
github-advanced-security[bot] marked this conversation as resolved.
Fixed
|
||||||||||||||
|
|
||||||||||||||
| on: | ||||||||||||||
| workflow_call: | ||||||||||||||
|
PratikWayase marked this conversation as resolved.
|
||||||||||||||
|
|
||||||||||||||
| permissions: | ||||||||||||||
| contents: read | ||||||||||||||
|
|
||||||||||||||
| jobs: | ||||||||||||||
| check-bare-pip-install: | ||||||||||||||
|
Check failure on line 10 in .github/workflows/pip-install-lint.yml
|
||||||||||||||
|
|
||||||||||||||
| name: Check for bare pip install | ||||||||||||||
| runs-on: ubuntu-latest | ||||||||||||||
| steps: | ||||||||||||||
| - name: Checkout code | ||||||||||||||
| uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 | ||||||||||||||
|
Comment on lines
+23
to
+24
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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.
Suggested change
|
||||||||||||||
|
|
||||||||||||||
| - name: Run bare pip install lint check | ||||||||||||||
| run: pwsh -File scripts/linting/Invoke-PipInstallLint.ps1 | ||||||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The scanner misses several maintained repository surfaces: |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,114 @@ | ||
| # Copyright (c) 2026 Microsoft Corporation. All rights reserved. | ||
| # SPDX-License-Identifier: MIT | ||
|
|
||
| <# | ||
| .SYNOPSIS | ||
| Lint script to detect bare 'pip install' calls. | ||
| The repository follows a 'uv-first' Python convention. | ||
| #> | ||
|
|
||
| param( | ||
| [string]$TestDirectory = "" | ||
|
PratikWayase marked this conversation as resolved.
Outdated
|
||
| ) | ||
|
|
||
| $ErrorActionPreference = "Stop" | ||
|
|
||
| $script:ExcludeDirs = @(".git", "evals", ".venv", "venv", "env", "node_modules", "__pycache__") | ||
| $script:ExcludeFiles = @("THIRD-PARTY-NOTICES", "Invoke-PipInstallLint.ps1", "Invoke-PipInstallLint.Tests.ps1") | ||
| $script:Violations = @() | ||
| $script:ScannedFiles = @{} | ||
|
|
||
| function script:Should-Exclude { | ||
| param([string]$Path) | ||
| $normalizedPath = $Path.Replace("\", "/").ToLowerInvariant() | ||
|
|
||
| foreach ($dir in $script:ExcludeDirs) { | ||
| if ($normalizedPath -match "(^|/)$dir(/|$)") { return $true } | ||
| } | ||
| foreach ($file in $script:ExcludeFiles) { | ||
| if ($normalizedPath -match "(^|/)$file(/|$)") { return $true } | ||
|
PratikWayase marked this conversation as resolved.
Outdated
|
||
| } | ||
| return $false | ||
| } | ||
|
|
||
| function script:Scan-File { | ||
| param([string]$FilePath) | ||
|
|
||
| $normalizedPath = $FilePath.Replace("\", "/") | ||
| if ($script:ScannedFiles.ContainsKey($normalizedPath)) { return } | ||
| $script:ScannedFiles[$normalizedPath] = $true | ||
|
|
||
| if (script:Should-Exclude -Path $FilePath) { return } | ||
|
|
||
| $ext = [System.IO.Path]::GetExtension($FilePath).ToLowerInvariant() | ||
| if ($ext -notin @(".py", ".ps1", ".yml", ".yaml", ".md", "")) { return } | ||
|
|
||
| try { | ||
| $lines = Get-Content -Path $FilePath -Raw -ErrorAction SilentlyContinue | ||
| if (-not $lines) { return } | ||
|
|
||
| $lineNumber = 1 | ||
| foreach ($line in $lines -split "`r?`n") { | ||
| $strippedLine = $line.Trim() | ||
|
|
||
| if ([string]::IsNullOrWhiteSpace($strippedLine)) { | ||
| $lineNumber++ | ||
| continue | ||
| } | ||
|
|
||
| if ($line -match "#\s*pip-install-ok\b" -or $line -match "<!--\s*pip-install-ok\s*-->") { | ||
| $lineNumber++ | ||
| continue | ||
| } | ||
|
|
||
| if ($line -match "\bpip3?\s+install\b" -and $line -notmatch "\buv\s+pip3?\s+install\b") { | ||
| if ($strippedLine -notmatch "^(name:|- name:)") { | ||
| $script:Violations += "$FilePath`:$lineNumber`: $strippedLine" | ||
| } | ||
| } | ||
| $lineNumber++ | ||
| } | ||
| } | ||
| catch { | ||
| Write-Warning "Could not read $FilePath`: $_" | ||
| } | ||
| } | ||
|
|
||
| function script:Invoke-Lint { | ||
| param([string]$TargetDir = ".") | ||
|
|
||
| $script:Violations = @() | ||
| $script:ScannedFiles = @{} | ||
|
|
||
| if ($TargetDir -eq ".") { | ||
| foreach ($dir in @(".github/workflows", "scripts")) { | ||
| if (Test-Path $dir) { | ||
| Get-ChildItem -Path $dir -Recurse -File -ErrorAction SilentlyContinue | ForEach-Object { script:Scan-File -FilePath $_.FullName } | ||
| } | ||
| } | ||
| Get-ChildItem -Path "." -Recurse -Include *.py, *.ps1, *.yml, *.yaml, *.md -File -ErrorAction SilentlyContinue | ForEach-Object { | ||
| script:Scan-File -FilePath $_.FullName | ||
| } | ||
| } else { | ||
| Get-ChildItem -Path $TargetDir -Recurse -Include *.py, *.ps1, *.yml, *.yaml, *.md -File -ErrorAction SilentlyContinue | ForEach-Object { | ||
| script:Scan-File -FilePath $_.FullName | ||
| } | ||
| } | ||
|
|
||
| if ($script:Violations.Count -gt 0) { | ||
| Write-Error "ERROR: Found bare 'pip install' calls. Use 'uv pip install' instead." | ||
| Write-Host "The repo follows a uv-first Python convention.`n" -ForegroundColor Yellow | ||
| foreach ($v in ($script:Violations | Sort-Object -Unique)) { | ||
| Write-Host " - $v" -ForegroundColor Red | ||
| } | ||
| return $false | ||
| } else { | ||
| Write-Host "Success: No bare 'pip install' calls found." -ForegroundColor Green | ||
| return $true | ||
| } | ||
| } | ||
|
|
||
| if ($MyInvocation.InvocationName -ne '.') { | ||
| $success = script:Invoke-Lint -TargetDir $TestDirectory | ||
| if (-not $success) { exit 1 } | ||
| } | ||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,78 @@ | ||||||||||||||||||||||||||||||||||||||||
| # Copyright (c) 2026 Microsoft Corporation. All rights reserved. | ||||||||||||||||||||||||||||||||||||||||
| # SPDX-License-Identifier: MIT | ||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||
| Describe "Invoke-PipInstallLint.ps1" { | ||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+1
to
+4
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nit: Add the standard Pester module requirement and
Suggested change
|
||||||||||||||||||||||||||||||||||||||||
| BeforeAll { | ||||||||||||||||||||||||||||||||||||||||
| $scriptPath = "$PSScriptRoot/../../linting/Invoke-PipInstallLint.ps1" | ||||||||||||||||||||||||||||||||||||||||
| . $scriptPath | ||||||||||||||||||||||||||||||||||||||||
| $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 | ||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+8
to
+19
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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
Suggested change
|
||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||
| It "Should pass on clean state" { | ||||||||||||||||||||||||||||||||||||||||
| $testFile = Join-Path $testDir "clean.py" | ||||||||||||||||||||||||||||||||||||||||
| Set-Content -Path $testFile -Value "print('hello world')" | ||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||
| $result = script:Invoke-Lint -TargetDir $testDir | ||||||||||||||||||||||||||||||||||||||||
| $result | Should -Be $true | ||||||||||||||||||||||||||||||||||||||||
| $script:Violations.Count | Should -Be 0 | ||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||
| It "Should detect bare pip install violation" { | ||||||||||||||||||||||||||||||||||||||||
| $testFile = Join-Path $testDir "violation.yml" | ||||||||||||||||||||||||||||||||||||||||
| Set-Content -Path $testFile -Value "run: pip install malicious-package" | ||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||
| $result = script:Invoke-Lint -TargetDir $testDir | ||||||||||||||||||||||||||||||||||||||||
| $result | Should -Be $false | ||||||||||||||||||||||||||||||||||||||||
| $script:Violations.Count | Should -BeGreaterThan 0 | ||||||||||||||||||||||||||||||||||||||||
| $script:Violations[0] | Should -Match "malicious-package" | ||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||
| It "Should respect exclusion logic (evals directory)" { | ||||||||||||||||||||||||||||||||||||||||
| $evalsDir = Join-Path $testDir "evals" | ||||||||||||||||||||||||||||||||||||||||
| New-Item -ItemType Directory -Path $evalsDir | Out-Null | ||||||||||||||||||||||||||||||||||||||||
| $testFile = Join-Path $evalsDir "fake_eval_test.py" | ||||||||||||||||||||||||||||||||||||||||
| Set-Content -Path $testFile -Value "run: pip install mock-package" | ||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||
| $result = script:Invoke-Lint -TargetDir $testDir | ||||||||||||||||||||||||||||||||||||||||
| $result | Should -Be $true | ||||||||||||||||||||||||||||||||||||||||
| $script:Violations.Count | Should -Be 0 | ||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||
| It "Should allow uv pip install" { | ||||||||||||||||||||||||||||||||||||||||
| $testFile = Join-Path $testDir "uv_allowed.py" | ||||||||||||||||||||||||||||||||||||||||
| Set-Content -Path $testFile -Value "run: uv pip install fastapi" | ||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||
| $result = script:Invoke-Lint -TargetDir $testDir | ||||||||||||||||||||||||||||||||||||||||
| $result | Should -Be $true | ||||||||||||||||||||||||||||||||||||||||
| $script:Violations.Count | Should -Be 0 | ||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||
| It "Should respect inline ignore marker for Python/YAML" { | ||||||||||||||||||||||||||||||||||||||||
| $testFile = Join-Path $testDir "ignored.py" | ||||||||||||||||||||||||||||||||||||||||
| Set-Content -Path $testFile -Value "run: pip install legacy-package # pip-install-ok" | ||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||
| $result = script:Invoke-Lint -TargetDir $testDir | ||||||||||||||||||||||||||||||||||||||||
| $result | Should -Be $true | ||||||||||||||||||||||||||||||||||||||||
| $script:Violations.Count | Should -Be 0 | ||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||
| It "Should respect inline ignore marker for Markdown" { | ||||||||||||||||||||||||||||||||||||||||
| $testFile = Join-Path $testDir "ignored.md" | ||||||||||||||||||||||||||||||||||||||||
| Set-Content -Path $testFile -Value "run: pip install legacy-package <!-- pip-install-ok -->" | ||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||
| $result = script:Invoke-Lint -TargetDir $testDir | ||||||||||||||||||||||||||||||||||||||||
| $result | Should -Be $true | ||||||||||||||||||||||||||||||||||||||||
| $script:Violations.Count | Should -Be 0 | ||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||
Uh oh!
There was an error while loading. Please reload this page.