From 3c0bb127e0ab8b8858e4facc7a9370b1021b2727 Mon Sep 17 00:00:00 2001 From: Mark Berry Date: Fri, 22 May 2026 08:18:16 -0500 Subject: [PATCH] fix(msh): Check 5 was failing for every npm project on fleet workstations MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Symptom: every Node.js project scan emitted a WARN line: "SuspiciousScripts check failed for : The property 'Name' cannot be found on this object." Users mistook these for findings and were generating noise calls. Root cause: under StrictMode Latest, accessing `.Name` (or `.Count`) on an EMPTY PSMemberInfoCollection throws. The `$obj.PSObject.Properties.Name -contains 'foo'` idiom relies on auto-member-expansion which strict-mode refuses when the collection has zero items. Many published npm packages ship `"scripts": {}` in their package.json (declared, no hooks) — the common case, not an edge case — which made $scripts.PSObject.Properties empty and threw at line 94 of Find-MshSuspiciousScripts. The whole project's Check 5 was killed for every engineer running the scanner. Fix: switch the three `.PSObject.Properties.Name -contains 'X'` accesses in Find-MshSuspiciousScripts to the indexer pattern `$obj.PSObject.Properties['X']` which returns either the property or $null, regardless of whether the collection is empty. Regression coverage: added two Pester cases — `{"scripts": {}}` (the real-world trigger) and `{}` (sibling edge case). All 15 tests in the suite pass with the fix. Latent same-pattern bugs in Find-MshBadPackages.ps1 (lines 286, 334, 335) operate on package.json `dependencies` / `devDependencies` sections that can also be `{}` in the wild. Not changed in this commit per smallest-safe-change discipline — flagged for follow-up. Co-Authored-By: Claude Opus 4.7 --- .../Find-MshSuspiciousScripts.ps1 | 28 ++++++++++++------- .../Find-MshSuspiciousScripts.Tests.ps1 | 20 +++++++++++++ 2 files changed, 38 insertions(+), 10 deletions(-) diff --git a/Private/MiniShaiHulud/Find-MshSuspiciousScripts.ps1 b/Private/MiniShaiHulud/Find-MshSuspiciousScripts.ps1 index 7c10910..0c5697c 100644 --- a/Private/MiniShaiHulud/Find-MshSuspiciousScripts.ps1 +++ b/Private/MiniShaiHulud/Find-MshSuspiciousScripts.ps1 @@ -76,13 +76,19 @@ function Find-MshSuspiciousScripts { if (-not $pkg) { continue } # Defensive: ConvertFrom-Json on a top-level scalar (e.g. just `42` - # or `"hello"`) returns a primitive, not a PSCustomObject. Under - # StrictMode Latest, $primitive.PSObject.Properties.Name throws - # because the empty PSMemberInfoCollection doesn't expose .Name as - # a direct property and the strict-mode property-fallback fails. - # Hard-gate on PSCustomObject to avoid the entire class of failures. + # or `"hello"`) returns a primitive, not a PSCustomObject. Hard-gate + # on PSCustomObject before any property access. if (-not ($pkg -is [System.Management.Automation.PSCustomObject])) { continue } - if (-not ($pkg.PSObject.Properties.Name -contains 'scripts')) { continue } + # Use the PSObject.Properties indexer instead of the `.Name -contains` + # idiom. Under StrictMode Latest, the `.Name` and `.Count` members on + # an EMPTY PSMemberInfoCollection throw "The property 'Name' cannot + # be found on this object" — and the empty-collection case is hit + # routinely by published npm packages that ship `"scripts": {}` in + # their package.json. The indexer returns either the property or + # $null, regardless of whether the collection is empty. See + # Tests/MiniShaiHulud/Find-MshSuspiciousScripts.Tests.ps1 — the + # "empty scripts object" regression case. + if ($null -eq $pkg.PSObject.Properties['scripts']) { continue } $scripts = $pkg.scripts if (-not $scripts) { continue } # 'scripts' value can legally be a string, array, or null in malformed @@ -91,8 +97,9 @@ function Find-MshSuspiciousScripts { if (-not ($scripts -is [System.Management.Automation.PSCustomObject])) { continue } foreach ($hook in @('postinstall', 'preinstall', 'install')) { - if (-not ($scripts.PSObject.Properties.Name -contains $hook)) { continue } - $rawHook = $scripts.$hook + $hookProp = $scripts.PSObject.Properties[$hook] + if ($null -eq $hookProp) { continue } + $rawHook = $hookProp.Value if ($null -eq $rawHook) { continue } $script = [string]$rawHook if ([string]::IsNullOrWhiteSpace($script)) { continue } @@ -105,8 +112,9 @@ function Find-MshSuspiciousScripts { $hasExec = ($script -match 'child_process') -or ($script -match '\bexec\b') -or ($script -match '\bspawn\b') $severity = if ($hasDecode -and $hasExec) { 'Critical' } else { 'High' } - $pkgName = if ($pkg.PSObject.Properties.Name -contains 'name' -and $pkg.name) { - [string]$pkg.name + $nameProp = $pkg.PSObject.Properties['name'] + $pkgName = if ($null -ne $nameProp -and $nameProp.Value) { + [string]$nameProp.Value } else { Split-Path (Split-Path $mf -Parent) -Leaf } diff --git a/Tests/MiniShaiHulud/Find-MshSuspiciousScripts.Tests.ps1 b/Tests/MiniShaiHulud/Find-MshSuspiciousScripts.Tests.ps1 index 1108e7f..1b65271 100644 --- a/Tests/MiniShaiHulud/Find-MshSuspiciousScripts.Tests.ps1 +++ b/Tests/MiniShaiHulud/Find-MshSuspiciousScripts.Tests.ps1 @@ -63,6 +63,26 @@ Describe 'Find-MshSuspiciousScripts — StrictMode resilience to malformed packa { Find-MshSuspiciousScripts -ProjectPath $script:proj -Iocs $script:iocs } | Should -Not -Throw } + It 'does not throw when scripts is an empty object {} (real-world common case)' { + # Many published npm packages ship `"scripts": {}` in package.json + # (declared, no hooks). Under StrictMode Latest, accessing + # $scripts.PSObject.Properties.Name on an EMPTY PSMemberInfoCollection + # throws "The property 'Name' cannot be found on this object." This + # bug was killing Check 5 once per project for every fleet engineer + # running the scanner — fixed by switching to the PSObject.Properties + # indexer pattern which is safe on empty collections. + _Plant 'empty-scripts' '{"name":"x","version":"1.0.0","scripts":{}}' + { Find-MshSuspiciousScripts -ProjectPath $script:proj -Iocs $script:iocs } | Should -Not -Throw + } + + It 'does not throw when package.json itself is an empty object {}' { + # Edge-case sibling of the empty-scripts bug: $pkg.PSObject.Properties + # is empty so any `.Name` access would throw. Fixed by the same + # indexer-pattern switch. + _Plant 'empty-pkg' '{}' + { Find-MshSuspiciousScripts -ProjectPath $script:proj -Iocs $script:iocs } | Should -Not -Throw + } + It 'does not throw when an individual hook value is null' { _Plant 'hook-null' '{"name":"x","scripts":{"postinstall":null,"build":"webpack"}}' { Find-MshSuspiciousScripts -ProjectPath $script:proj -Iocs $script:iocs } | Should -Not -Throw