Skip to content

Commit 6b5ae06

Browse files
WilliamBerryiiiBill Berry
WilliamBerryiii
and
Bill Berry
authored
feat(scripts): add workflow npm command scanning to dependency pinning (#837)
This PR adds **workflow npm command scanning** to the dependency pinning security scanner. The new `workflow-npm-commands` pattern type detects `npm install`, `npm update`, `npm i`, and `npm install-test` commands inside GitHub Actions workflow `run:` blocks, reporting each as a Medium-severity dependency pinning violation. These commands are security-relevant because they resolve unpinned versions at install time, unlike `npm ci` which reads from a lockfile. ## Description Three new functions were added to *Test-DependencyPinning.ps1* to implement the scanning capability: - **`Get-WorkflowNpmCommandViolations`** parses workflow YAML files using an indentation-aware state machine that tracks `run:` block boundaries, distinguishes inline versus multi-line (pipe/folded) blocks, and correctly skips comment lines. - **`Test-NpmCommandLine`** applies two regex patterns to each line: `'\bnpm\s+(install-test|install|update)\b'` for full command names and `'\bnpm\s+i\b(?!nstall|nit)'` for the short alias with a defensive negative lookahead. - **`New-NpmCommandViolation`** constructs `DependencyViolation` objects with all required fields, using Medium severity and actionable remediation messages matching existing conventions. The `workflow-npm-commands` entry was added to the `$DependencyPatterns` hashtable, and `$IncludeTypes` defaults were updated in both the script-level parameter block and the `Invoke-DependencyPinningAnalysis` function parameter block. Seven Pester test cases were added to *Test-DependencyPinning.Tests.ps1* covering violation detection (4 expected hits), safe command exclusion, commented code handling, mixed blocks, and nonexistent file error paths. Two fixture files provide realistic GitHub Actions workflow structures for testing. ## Related Issue(s) Related to #525 ## Type of Change Select all that apply: **Code & Documentation:** * [ ] Bug fix (non-breaking change fixing an issue) * [x] New feature (non-breaking change adding functionality) * [ ] Breaking change (fix or feature causing existing functionality to change) * [ ] Documentation update **Infrastructure & Configuration:** * [ ] GitHub Actions workflow * [ ] Linting configuration (markdown, PowerShell, etc.) * [ ] Security configuration * [ ] DevContainer configuration * [ ] Dependency update **AI Artifacts:** * [ ] Reviewed contribution with `prompt-builder` agent and addressed all feedback * [ ] Copilot instructions (`.github/instructions/*.instructions.md`) * [ ] Copilot prompt (`.github/prompts/*.prompt.md`) * [ ] Copilot agent (`.github/agents/*.agent.md`) * [ ] Copilot skill (`.github/skills/*/SKILL.md`) **Other:** * [x] Script/automation (`.ps1`, `.sh`, `.py`) * [ ] Other (please describe): ## Testing - 81/81 Pester tests pass, including 7 new test cases in the `Get-WorkflowNpmCommandViolations` Describe block. - 0 PSScriptAnalyzer issues across 69 files. - Test cases cover nominal detection (4 violations from *workflow-npm-install.yml*), negative testing (0 violations from *workflow-npm-ci-only.yml*), commented `npm install` lines, mixed safe/unsafe blocks, and nonexistent file handling. ## Checklist ### Required Checks * [ ] Documentation is updated (if applicable) * [x] Files follow existing naming conventions * [x] Changes are backwards compatible (if applicable) * [x] Tests added for new functionality (if applicable) ### Required Automated Checks The following validation commands must pass before merging: * [x] Markdown linting: `npm run lint:md` * [x] Spell checking: `npm run spell-check` * [x] Frontmatter validation: `npm run lint:frontmatter` * [x] Skill structure validation: `npm run validate:skills` * [x] Link validation: `npm run lint:md-links` * [x] PowerShell analysis: `npm run lint:ps` Co-authored-by: Bill Berry <wbery@microsoft.com>
1 parent ca1e65b commit 6b5ae06

File tree

4 files changed

+321
-2
lines changed

4 files changed

+321
-2
lines changed

‎scripts/security/Test-DependencyPinning.ps1‎

Lines changed: 159 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -109,7 +109,7 @@ param(
109109
[string]$ExcludePaths = "",
110110

111111
[Parameter(Mandatory = $false)]
112-
[string]$IncludeTypes = "github-actions,npm,pip,shell-downloads",
112+
[string]$IncludeTypes = "github-actions,npm,pip,shell-downloads,workflow-npm-commands",
113113

114114
[Parameter(Mandatory = $false)]
115115
[ValidateRange(0, 100)]
@@ -165,12 +165,169 @@ $DependencyPatterns = @{
165165
ValidationFunc = 'Test-ShellDownloadSecurity'
166166
Description = 'Shell script downloads must include checksum verification'
167167
}
168+
169+
'workflow-npm-commands' = @{
170+
FilePatterns = @('**/.github/workflows/*.yml', '**/.github/workflows/*.yaml')
171+
ValidationFunc = 'Get-WorkflowNpmCommandViolations'
172+
Description = 'Workflow npm install/update commands should use npm ci'
173+
}
168174
}
169175

170176
# DependencyViolation and ComplianceReport classes moved to ./Modules/SecurityClasses.psm1
171177

172178
#region Functions
173179

180+
function Test-NpmCommandLine {
181+
<#
182+
.SYNOPSIS
183+
Tests whether a line contains an unpinned npm command.
184+
.DESCRIPTION
185+
Matches npm install, npm i, npm update, and npm install-test commands.
186+
Does not match npm ci, npm run, npm test, npm audit, or npx.
187+
.PARAMETER Line
188+
The text line to test for npm commands.
189+
.OUTPUTS
190+
System.String or $null
191+
#>
192+
param(
193+
[Parameter(Mandatory)]
194+
[string]$Line
195+
)
196+
197+
if ($Line -match '\bnpm\s+(install-test|install|update)\b') {
198+
return $Matches[0]
199+
}
200+
if ($Line -match '\bnpm\s+i\b(?!nstall|nit)') {
201+
return $Matches[0]
202+
}
203+
204+
return $null
205+
}
206+
207+
function New-NpmCommandViolation {
208+
<#
209+
.SYNOPSIS
210+
Creates a DependencyViolation for an unpinned npm command.
211+
.DESCRIPTION
212+
Constructs a DependencyViolation object with standard fields for
213+
npm command violations detected in workflow run: steps.
214+
.PARAMETER FileInfo
215+
Hashtable with Path, Type, and RelativePath keys.
216+
.PARAMETER LineNumber
217+
1-based line number of the violation.
218+
.PARAMETER Line
219+
The source line containing the npm command.
220+
.PARAMETER Command
221+
The matched npm command string.
222+
.OUTPUTS
223+
DependencyViolation
224+
#>
225+
param(
226+
[Parameter(Mandatory)]
227+
[hashtable]$FileInfo,
228+
[Parameter(Mandatory)]
229+
[int]$LineNumber,
230+
[Parameter(Mandatory)]
231+
[string]$Line,
232+
[Parameter(Mandatory)]
233+
[string]$Command
234+
)
235+
236+
$violation = [DependencyViolation]::new(
237+
$FileInfo.RelativePath,
238+
$LineNumber,
239+
'workflow-npm-commands',
240+
$Command,
241+
'Medium',
242+
"Unpinned npm command detected: '$Command'. Use 'npm ci' for deterministic installs from lockfile."
243+
)
244+
$violation.ViolationType = 'Unpinned'
245+
$violation.CurrentRef = $Line.Trim()
246+
$violation.Remediation = "Replace '$Command' with 'npm ci' for reproducible builds."
247+
return $violation
248+
}
249+
250+
function Get-WorkflowNpmCommandViolations {
251+
<#
252+
.SYNOPSIS
253+
Detects unpinned npm install commands in GitHub Actions workflow run: steps.
254+
.DESCRIPTION
255+
Scans workflow YAML files for run: blocks and detects npm commands that
256+
modify the dependency tree (install, i, update, install-test). Commands
257+
that use the lockfile deterministically (ci) or do not install packages
258+
(run, test, audit) are not flagged.
259+
260+
Uses indentation-aware parsing to confine detection to actual run: block
261+
content, reducing false positives from YAML comments or unrelated keys.
262+
.PARAMETER FileInfo
263+
Hashtable with Path, Type, and RelativePath keys identifying the file to scan.
264+
.OUTPUTS
265+
DependencyViolation[]
266+
#>
267+
param(
268+
[Parameter(Mandatory)]
269+
[hashtable]$FileInfo
270+
)
271+
272+
$violations = @()
273+
$filePath = $FileInfo.Path
274+
275+
if (-not (Test-Path -LiteralPath $filePath)) {
276+
return $violations
277+
}
278+
279+
$lines = Get-Content -LiteralPath $filePath
280+
$inRunBlock = $false
281+
$runBlockIndent = 0
282+
283+
for ($i = 0; $i -lt $lines.Count; $i++) {
284+
$line = $lines[$i]
285+
$trimmed = $line.TrimStart()
286+
287+
if ($trimmed -eq '' -or $trimmed.StartsWith('#')) {
288+
continue
289+
}
290+
291+
$currentIndent = $line.Length - $line.TrimStart().Length
292+
293+
if ($trimmed -match '^run:\s*(.*)$') {
294+
$runContent = $Matches[1].Trim()
295+
$runBlockIndent = $currentIndent
296+
297+
if ($runContent -and $runContent -notmatch '^[|>]') {
298+
$npmMatch = Test-NpmCommandLine -Line $runContent
299+
if ($npmMatch) {
300+
$violations += New-NpmCommandViolation -FileInfo $FileInfo -LineNumber ($i + 1) -Line $runContent -Command $npmMatch
301+
}
302+
$inRunBlock = $false
303+
} else {
304+
$inRunBlock = $true
305+
}
306+
continue
307+
}
308+
309+
if ($inRunBlock) {
310+
if ($currentIndent -le $runBlockIndent) {
311+
$inRunBlock = $false
312+
if ($trimmed -match '^run:\s*(.*)$') {
313+
$i--
314+
continue
315+
}
316+
} else {
317+
if ($trimmed.StartsWith('#')) {
318+
continue
319+
}
320+
$npmMatch = Test-NpmCommandLine -Line $trimmed
321+
if ($npmMatch) {
322+
$violations += New-NpmCommandViolation -FileInfo $FileInfo -LineNumber ($i + 1) -Line $trimmed -Command $npmMatch
323+
}
324+
}
325+
}
326+
}
327+
328+
return $violations
329+
}
330+
174331
function Test-ShellDownloadSecurity {
175332
<#
176333
.SYNOPSIS
@@ -795,7 +952,7 @@ function Invoke-DependencyPinningAnalysis {
795952
[switch]$Recursive,
796953

797954
[Parameter()]
798-
[string]$IncludeTypes = "github-actions,npm,pip,shell-downloads",
955+
[string]$IncludeTypes = "github-actions,npm,pip,shell-downloads,workflow-npm-commands",
799956

800957
[Parameter()]
801958
[string]$ExcludePaths = "",

‎scripts/tests/Fixtures/Workflows/workflow-npm-ci-only.yml‎

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
name: test-npm-ci-only
2+
on: push
3+
jobs:
4+
build:
5+
runs-on: ubuntu-latest
6+
steps:
7+
- uses: actions/checkout@v4
8+
- name: Install from lockfile
9+
run: npm ci
10+
- name: Run linting
11+
run: npm run lint
12+
- name: Run tests
13+
run: npm test
14+
- name: Security audit
15+
run: npm audit --audit-level=moderate
16+
- name: Check version
17+
run: npm --version
18+
- name: Multi-step clean
19+
run: |
20+
npm ci
21+
npm run build
22+
npm run test

‎scripts/tests/Fixtures/Workflows/workflow-npm-install.yml‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
name: test-npm-install
2+
on: push
3+
jobs:
4+
build:
5+
runs-on: ubuntu-latest
6+
steps:
7+
- uses: actions/checkout@v4
8+
- name: Install dependencies
9+
run: npm install
10+
- name: Update packages
11+
run: |
12+
npm update
13+
npm run build
14+
- name: Install and test
15+
run: npm install-test
16+
- name: Short alias install
17+
run: npm i --save-dev some-package

‎scripts/tests/security/Test-DependencyPinning.Tests.ps1‎

Lines changed: 123 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1171,3 +1171,126 @@ Describe 'Invoke-DependencyPinningAnalysis' -Tag 'Unit' {
11711171
}
11721172
}
11731173
}
1174+
1175+
Describe 'Get-WorkflowNpmCommandViolations' -Tag 'Unit' {
1176+
BeforeAll {
1177+
# Source the script to get functions
1178+
. $PSScriptRoot/../../security/Test-DependencyPinning.ps1
1179+
1180+
$script:fixtureDir = Join-Path $PSScriptRoot '../Fixtures/Workflows'
1181+
}
1182+
1183+
Context 'when workflow contains npm install commands' {
1184+
It 'should detect npm install in single-line run step' {
1185+
$fileInfo = @{
1186+
Path = Join-Path $script:fixtureDir 'workflow-npm-install.yml'
1187+
Type = 'workflow-npm-commands'
1188+
RelativePath = 'workflow-npm-install.yml'
1189+
}
1190+
$violations = Get-WorkflowNpmCommandViolations -FileInfo $fileInfo
1191+
$violations | Should -HaveCount 4
1192+
}
1193+
1194+
It 'should return DependencyViolation objects' {
1195+
$fileInfo = @{
1196+
Path = Join-Path $script:fixtureDir 'workflow-npm-install.yml'
1197+
Type = 'workflow-npm-commands'
1198+
RelativePath = 'workflow-npm-install.yml'
1199+
}
1200+
$violations = Get-WorkflowNpmCommandViolations -FileInfo $fileInfo
1201+
$violations | ForEach-Object {
1202+
$_.GetType().Name | Should -Be 'DependencyViolation'
1203+
$_.Type | Should -Be 'workflow-npm-commands'
1204+
$_.Severity | Should -Be 'Medium'
1205+
$_.ViolationType | Should -Be 'Unpinned'
1206+
}
1207+
}
1208+
1209+
It 'should report accurate line numbers' {
1210+
$fileInfo = @{
1211+
Path = Join-Path $script:fixtureDir 'workflow-npm-install.yml'
1212+
Type = 'workflow-npm-commands'
1213+
RelativePath = 'workflow-npm-install.yml'
1214+
}
1215+
$violations = Get-WorkflowNpmCommandViolations -FileInfo $fileInfo
1216+
$violations | ForEach-Object {
1217+
$_.Line | Should -BeGreaterThan 0
1218+
}
1219+
}
1220+
}
1221+
1222+
Context 'when workflow contains only safe npm commands' {
1223+
It 'should return no violations for npm ci and npm run' {
1224+
$fileInfo = @{
1225+
Path = Join-Path $script:fixtureDir 'workflow-npm-ci-only.yml'
1226+
Type = 'workflow-npm-commands'
1227+
RelativePath = 'workflow-npm-ci-only.yml'
1228+
}
1229+
$violations = Get-WorkflowNpmCommandViolations -FileInfo $fileInfo
1230+
$violations | Should -HaveCount 0
1231+
}
1232+
}
1233+
1234+
Context 'when file does not exist' {
1235+
It 'should return empty array' {
1236+
$fileInfo = @{
1237+
Path = '/tmp/nonexistent-workflow.yml'
1238+
Type = 'workflow-npm-commands'
1239+
RelativePath = 'nonexistent.yml'
1240+
}
1241+
$violations = Get-WorkflowNpmCommandViolations -FileInfo $fileInfo
1242+
$violations | Should -HaveCount 0
1243+
}
1244+
}
1245+
1246+
Context 'edge cases with inline test data' {
1247+
It 'should not flag commented-out npm install' {
1248+
$yaml = @'
1249+
name: test
1250+
on: push
1251+
jobs:
1252+
build:
1253+
runs-on: ubuntu-latest
1254+
steps:
1255+
- name: Build
1256+
run: |
1257+
# npm install
1258+
npm ci
1259+
'@
1260+
$tempFile = Join-Path $TestDrive 'commented-npm.yml'
1261+
Set-Content -Path $tempFile -Value $yaml
1262+
$fileInfo = @{
1263+
Path = $tempFile
1264+
Type = 'workflow-npm-commands'
1265+
RelativePath = 'commented-npm.yml'
1266+
}
1267+
$violations = Get-WorkflowNpmCommandViolations -FileInfo $fileInfo
1268+
$violations | Should -HaveCount 0
1269+
}
1270+
1271+
It 'should detect npm install in multi-line block alongside safe commands' {
1272+
$yaml = @'
1273+
name: test
1274+
on: push
1275+
jobs:
1276+
build:
1277+
runs-on: ubuntu-latest
1278+
steps:
1279+
- name: Setup
1280+
run: |
1281+
npm install
1282+
npm run build
1283+
'@
1284+
$tempFile = Join-Path $TestDrive 'mixed-npm.yml'
1285+
Set-Content -Path $tempFile -Value $yaml
1286+
$fileInfo = @{
1287+
Path = $tempFile
1288+
Type = 'workflow-npm-commands'
1289+
RelativePath = 'mixed-npm.yml'
1290+
}
1291+
$violations = Get-WorkflowNpmCommandViolations -FileInfo $fileInfo
1292+
$violations | Should -HaveCount 1
1293+
$violations[0].Name | Should -BeLike 'npm install*'
1294+
}
1295+
}
1296+
}

0 commit comments

Comments
 (0)