feat(security): add PSGallery staleness check and BOM cleanup#1379
feat(security): add PSGallery staleness check and BOM cleanup#1379WilliamBerryiii merged 13 commits intomainfrom
Conversation
…t files - add Get-PSModuleStaleness in Test-SHAStaleness.ps1 querying PowerShell Gallery OData API for psModules pinned in tool-checksums.json - add Get-PSGalleryApiBase helper in SecurityHelpers.psm1 honoring HVE_PSGALLERY_REPOSITORY override - add psModules array to tool-checksums.json (PowerShell-Yaml 0.4.7, Pester 5.7.1, PSScriptAnalyzer 1.25.0) plus per-arch SHAs for actionlint and gitleaks - expand Test-SHAStaleness.Tests.ps1 with Get-PSModuleStaleness coverage and PSGallery mocks (63/63 pass) - strip UTF-8 BOM from 11 PowerShell test/helper files 🔒 - Generated by Copilot
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
PR Review: feat(security): add PSGallery staleness check and BOM cleanup
The implementation work here is solid — the PSGallery OData integration is well-structured, the HVE_PSGALLERY_REPOSITORY override for offline/test use is a thoughtful design decision, the BOM cleanup is correct, and the 63-test coverage is comprehensive. There are two blocking template compliance issues and two coding standards violations that need to be addressed before merging.
🔴 Issue Alignment
No issue is linked with Fixes # or Closes #. The PR description's Related section points to PR #1378 (not an issue), describing this as a follow-up. The template requires a linked issue. If this work stems from a review advisory in #1378 rather than a standalone issue, please either:
- Open a tracking issue for the advisory and link it here, or
- Add a note explaining the intentional absence of an issue (e.g., "No issue; implements deferred advisory from PR #1378")
🔴 PR Template Compliance
The PR description uses a custom heading structure (## Summary, ## Changes, ## Validation, ## Related) rather than the repository's PR template. None of the required template checkboxes are filled in:
- Type of Change — no box is checked. This PR is a Script/automation change (
.ps1/.psm1modifications) and could also check Security configuration given thetool-checksums.jsonupdate. - Testing — the Validation section documents test results, but the template Testing section and its automated-checks checklist are absent.
- Checklist — Required Checks and Required Automated Checks are not present.
- Security Considerations — the sensitive-data and dependency-security checkboxes are not checked.
Please reformat the description to include the standard template sections and check the applicable boxes.
⚠️ Coding Standards Violations
Two PSScriptAnalyzer / Pester convention issues were found in the new code (see inline comments):
Get-PSModuleStalenessis missing[OutputType([PSCustomObject[]])](scripts/security/Test-SHAStaleness.ps1, line 738) —PSUseOutputTypeCorrectlywill flag this innpm run lint:ps.Describe 'Get-PSModuleStaleness'is missing-Tag 'Unit'(scripts/tests/security/Test-SHAStaleness.Tests.ps1, line 234) — allDescribeblocks must carry a tag per the Pester instructions.
A third, lower-priority suggestion is noted inline for the Get-PSGalleryApiBase help block in SecurityHelpers.psm1.
✅ Code Quality Observations
- The OData response handling covers both single-entry and feed-wrapped response shapes — good defensive coding.
$errorModulesare logged as warnings but intentionally excluded from$script:StaleDependencies; this distinction (check error ≠ stale dependency) is correct.- The
sha256ByArch/assetTemplateByArchadditions totool-checksums.jsonare well-structured and thegitleaksvendor naming note is a helpful clarification. #Requires -Modules @{ ModuleName='PowerShell-Yaml'; RequiredVersion='0.4.7' }pinning is consistent with the dependency-pinning philosophy.
Action Items
- ❌ Fill in the PR template (Type of Change, Testing, Checklist, Security Considerations).
- ❌ Add a linked issue or document the absence of one.
⚠️ Add[OutputType([PSCustomObject[]])]toGet-PSModuleStaleness.⚠️ Add-Tag 'Unit'to the newDescribe 'Get-PSModuleStaleness'block.
|
To reviewer's ... please note this is a follow-on from #1378 and build off that branch's work. |
…uery - Get-PSModuleStaleness rejects entries missing 'name' or 'version' before constructing PSGallery URI - Malformed entries surface as High-severity StaleDependencies so -FailOnStale catches manifest bugs - Network errors remain warnings to avoid flaky CI on transient PSGallery outages
There was a problem hiding this comment.
Review Summary
This PR adds PSGallery staleness checking to Test-SHAStaleness.ps1, exports Get-PSGalleryApiBase from SecurityHelpers.psm1, extends tool-checksums.json with psModules entries and per-arch fields, and removes UTF-8 BOMs from 11 PowerShell files. The implementation direction is sound, and the 63-passing test suite is a strong baseline — but several items must be addressed before merge.
🔗 Issue Alignment
No linked issue. The PR description references PR #1378 as a follow-up but does not link to a GitHub Issue using Fixes # or Closes #. The PR template requires a valid issue reference in the Related Issue(s) section. If this work is self-contained and tracked via the advisory in #1378, please open a tracking issue or add an explicit cross-reference using the required syntax.
📋 PR Template Compliance
The PR description does not follow the repository's pull request template:
- ❌ No
## Descriptionsection — the PR opens with## Summaryinstead of the template's## Descriptionheading. - ❌ No
## Related Issue(s)section —#1378appears under a non-standard## Relatedheading without aFixes/Closeskeyword. - ❌ No
## Type of Changecheckboxes — none of the template checkboxes are present or checked (e.g.,[x] Script/automation,[x] Security configuration). - ❌ No
## Checklistsection — required automated checks (npm run lint:ps,npm run lint:md, etc.) are not attested. - ❌ No
## Security Considerationssection — the template's security attestation checkboxes are absent.
Please rewrite the PR description using the .github/PULL_REQUEST_TEMPLATE.md template and fill in all applicable sections.
🔧 Coding Standards
Two objective violations of repository conventions were found in the new code. See inline comments for details and suggested fixes:
-
[OutputType]missing onGet-PSModuleStaleness(scripts/security/Test-SHAStaleness.ps1line 738) — violatesps-script.instructions.mdand will failnpm run lint:ps(PSScriptAnalyzer rulePSUseOutputTypeCorrectly). -
-Tag 'Unit'missing onDescribe 'Get-PSModuleStaleness'(scripts/tests/security/Test-SHAStaleness.Tests.ps1line 234) — violatespester-test.instructions.md; everyDescribeblock must carry-Tag 'Unit'.
🔍 Code Quality
-
Undocumented version-pin changes in BOM-cleanup files —
scripts/collections/Modules/CollectionHelpers.psm1andscripts/extension/Find-CollectionManifests.ps1are described as BOM-only changes but each also addsRequiredVersion='0.4.7'to#Requires -Modules. This functional change should be documented in the PR description. (See inline comment onCollectionHelpers.psm1.) -
Misleading
$errorModuleswarning count — The "Failed to check N PowerShell module(s)" log message aggregates both network failures and malformed manifest entries. The two failure modes are distinct; the count should exclude malformed entries so the message remains accurate. (See inline comment at line 931.)
✅ What Looks Good
Get-PSGalleryApiBasefollows the exact same shape asGet-GitHubApiBaseand correctly honours theHVE_PSGALLERY_REPOSITORYoverride for offline/test use.tool-checksums.jsonadditions are well-structured; the per-archsha256ByArch/assetTemplateByArchfields are a clean forward-compatible extension.- The
Get-PSModuleStalenessfunction handles missing manifest, absentpsModulesarray, missing name/version fields, network errors, and version comparison throughCompare-ToolVersion— all the expected paths are covered. - Test coverage for the new function is thorough (mocked PSGallery, absent manifest, error capture, stale/non-stale differentiation).
Action Items
- Rewrite the PR description to conform to the repository template (Description, Related Issue(s) with
Fixes #, Type of Change checkboxes, Checklist, Security Considerations). - Link or create a GitHub Issue for this work.
- Add
[OutputType([object[]])]toGet-PSModuleStaleness. - Add
-Tag 'Unit'toDescribe 'Get-PSModuleStaleness'. - Document the
#Requiresversion-pin changes in the PR description. - Tighten the
$errorModulesfilter to exclude malformed entries from the "Failed to check" log message.
- add UTF-8 BOM to 8 Pester test files to satisfy PSUseBOMForUnicodeEncodedFile - pin PowerShell-Yaml install to RequiredVersion 0.4.7 in 9 workflows to match #Requires - remove -ListAvailable short-circuit guards that blocked version-correction installs 🔧 - Generated by Copilot
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1379 +/- ##
==========================================
- Coverage 87.62% 87.60% -0.02%
==========================================
Files 62 62
Lines 9696 9790 +94
==========================================
+ Hits 8496 8577 +81
- Misses 1200 1213 +13
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Advisory review — this PR is from a maintainer. Findings are informational only.
Overview
This PR extends Test-SHAStaleness.ps1 with Get-PSModuleStaleness to track pinned PowerShell Gallery modules, adds psModules entries to tool-checksums.json, exports Get-PSGalleryApiBase from SecurityHelpers.psm1, and strips UTF-8 BOM bytes from 11 test/helper files. The overall direction is solid and the BOM cleanup is a clean housekeeping win.
Three advisory items are filed as inline comments; two additional process observations are noted below.
Issue Alignment
No formal issue is linked. The PR description references PR #1378 ("Follow-up to #1378") but does not use Fixes #, Closes #, or Resolves # to link a tracked issue. If the work was captured in a GitHub Issue, please add the standard link so automated issue-closure and project-board tracking can fire correctly.
PR Template Compliance
The PR body uses custom section headers (## Summary, ## Changes, ## Validation, ## Related) rather than the sections defined in .github/PULL_REQUEST_TEMPLATE.md. The following required sections are absent from the submitted description:
- Type of Change — no checkboxes selected (the PR involves
Script/automation,Security configuration, and likelyGitHub Actions workflowfrom the workflow pins carried over from #1378). - Checklist — Required Checks and Required Automated Checks checkboxes are missing. The description does record the Pester test run result, which is helpful, but the full checklist (
lint:md,lint:ps,lint:frontmatter, etc.) is not present. - Security Considerations — attestation checkboxes are absent.
Coding Standards
See inline comments for:
Get-PSModuleStaleness(line 738 ·scripts/security/Test-SHAStaleness.ps1): Missing[OutputType([PSCustomObject[]])]— required byPSUseOutputTypeCorrectly.Describe 'Get-PSModuleStaleness'(line 234 ·scripts/tests/security/Test-SHAStaleness.Tests.ps1): Missing-Tag 'Unit'— required by Pester conventions; the block would be silently excluded from standard tag-filtered CI runs.
Code Quality
See inline comment on lines 779–783 of scripts/security/Test-SHAStaleness.ps1:
- OData response structure mismatch: The PSGallery v2 endpoint returns Atom XML whose PowerShell-parsed property path (
$response.feed.entry.content.properties.Version) does not match either branch checked in the code. The unit-test mocks return a flat hashtable that satisfies the first branch, masking the production-path gap. This would causeIsStale = $nullfor every module against the live API.
Additional Observations
sha256ByArch/assetTemplateByArchfields added totool-checksums.jsonforactionlintandgitleaksare not yet consumed by any code in this PR. If these fields are forward-looking infrastructure for a future arch-aware download path, a brief comment in the JSON (or a follow-up issue) would help reviewers understand the intent and avoid them being treated as dead data.PSScriptAnalyzeris tracked inpsModulesat version1.25.0for staleness monitoring, but itsInstall-Modulecall incopilot-setup-steps.ymlis still unpinned (Install-Module -Name PSScriptAnalyzer -Force -Scope CurrentUser). This is consistent with the PR's stated scope (staleness detection, not necessarily pinning all modules in this PR), but worth a follow-up to close the loop.
- add [OutputType([PSCustomObject[]])] attribute to Get-PSModuleStaleness - exclude malformed manifest entries from PSGallery error count to avoid double-counting with malformedModules High-severity findings - tag Get-PSModuleStaleness Pester Describe block with -Tag 'Unit' for tag-filtered runs 🔧 - Generated by Copilot
There was a problem hiding this comment.
Advisory review, this PR is from a maintainer. Findings are informational only.
Review Summary
This PR is well-structured and the core feature — extending the SHA staleness validator to cover pinned PowerShell Gallery modules — is implemented cleanly. The test coverage is thorough (63/63 pass), the mock patterns align with the repository's Pester conventions, and the Get-PSGalleryApiBase abstraction correctly honours the HVE_PSGALLERY_REPOSITORY override for offline/test environments. The PowerShell-Yaml pinning changes across all workflow files are correct and consistent.
Three informational observations are raised as inline comments.
Issue Alignment
Fixes #, Closes #, or Resolves # reference to a tracked GitHub Issue. Issue #1377 (closed by #1378) could be relevant as the upstream tracking item, but this PR has no explicit link to any issue. This prevents GitHub from automatically closing work items on merge.
PR Template Compliance
.github/PULL_REQUEST_TEMPLATE.md.
The template requires these sections with checkboxes; this PR substitutes a different structure:
| Template Section | Status |
|---|---|
## Description |
❌ PR uses ## Summary |
## Related Issue(s) |
❌ PR uses ## Related without Fixes/Closes syntax |
## Type of Change (checkboxes) |
❌ Not present |
## Testing |
✅ Provided as ## Validation |
## Checklist (with sub-sections) |
❌ Not present |
Coding Standards
Three inline comments address specific findings. Summary:
-
scripts/security/tool-checksums.jsonline 11 — Dual schema fields (sha256/downloadUrlTemplateretained alongside newsha256ByArch/assetTemplateByArch) without documentation of precedence or migration intent. -
scripts/security/Test-SHAStaleness.ps1line 780 — PSGallery OData response parsing does not handle the array-feed shape; a silent no-version path could result if the API returns multipleentrychildren. -
.github/workflows/copilot-setup-steps.ymlline 88 —PSScriptAnalyzeris now tracked intool-checksums.jsonat1.25.0for staleness checking but remains unpinned in the workflowInstall-Modulecall (and in all other workflow files that install it), creating an inconsistency between the manifest and actual install behaviour.
Code Quality
✅ No bugs or logic errors identified.
✅ Error handling in Get-PSModuleStaleness is consistent — every failure path returns a structured result with a populated Error field rather than throwing, which integrates cleanly with the caller's collection loop.
✅ The $script:StaleDependencies integration in Invoke-SHAStalenessCheck correctly separates malformed manifest entries (elevated to High severity) from version staleness findings (Medium).
EF BB BF) in the repository:
scripts/tests/collections/CollectionHelpers.Tests.ps1scripts/tests/linting/Invoke-LinkLanguageCheck.Tests.ps1scripts/tests/linting/Validate-MarkdownFrontmatter.Tests.ps1scripts/tests/linting/Validate-SkillStructure.Tests.ps1scripts/tests/plugins/PluginHelpers.Tests.ps1scripts/tests/release/Update-VersionFiles.Tests.ps1scripts/tests/security/Invoke-PipAudit.Tests.ps1scripts/tests/security/Test-DependencyPinning.Tests.ps1
If this cleanup was intentional, a follow-up commit or PR to cover the remaining files would complete it.
Action Items (informational)
- Consider linking a GitHub Issue using
Fixes #orCloses #for traceability - Follow the PR template structure for future PRs in this repository
- Evaluate pinning
PSScriptAnalyzerto1.25.0in all workflowInstall-Modulecalls to match the manifest entry - Complete BOM cleanup for the 8 files still containing BOMs, or clarify the PR description
- Consider documenting the intended precedence of
sha256ByArchvs. legacysha256fields intool-checksums.json
- add .DESCRIPTION explaining the OData v2 PSGallery API base URL - add .NOTES describing OData query composition and Get-GitHubApiBase parity - resolve PR #1379 review comment 3107278707 📝 - Generated by Copilot
- defensively unwrap $response.entry when it arrives as an array - prevents silent null $latestVersion for multi-entry feed responses - resolve PR #1379 review comment 3107403490 📝 - Generated by Copilot
There was a problem hiding this comment.
Advisory review — this PR is from a maintainer. Findings are informational only.
Review Summary
The implementation is well-structured and thoroughly tested (63 Pester tests, live PSGallery validation). The PSGallery staleness function follows the existing Get-ToolStaleness pattern closely, error handling is defensive, and the dual-shape OData response parsing is correctly implemented. BOM cleanup and version pinning are clean, targeted changes.
Two inline advisory notes are attached. Additional template observations follow.
Issue Alignment
No tracking issue is linked. The PR explains it implements deferred advisories from PR #1378 and resolves BOM lint findings surfaced there. Flagging for awareness: the PR template requires a linked issue (Fixes #, Closes #, or Resolves #). Consider opening a follow-up issue or retroactively linking one if this work is important to surface in the changelog or project board.
PR Template Compliance
"GitHub Actions workflow" checkbox not checked — eight workflow files were modified (.github/workflows/*.yml) to pin PowerShell-Yaml to 0.4.7. The Infrastructure & Configuration → GitHub Actions workflow checkbox in the Type of Change section should be checked.
Required Automated Checks — the PR body checks three of the eight required items (lint:ps, lint:md, lint:frontmatter). The remaining five (spell-check, validate:skills, lint:md-links, plugin:generate, docs:test) are unchecked. If these passed locally and are not applicable to this change, they should still be checked with a note, or the results should be reported to satisfy the checklist intent.
Coding Standards
All PowerShell conventions are followed: copyright headers, [CmdletBinding()], [OutputType()], comment-based help, Export-ModuleMember, proper use of $ErrorActionPreference, and consistent naming. The Get-PSGalleryApiBase function mirrors Get-GitHubApiBase cleanly.
Code Quality
Two inline notes are attached:
scripts/security/Test-SHAStaleness.ps1line 776 — OData filter injection risk from unescaped module name (low severity, defensive fix suggested)..github/workflows/copilot-setup-steps.ymlline 91 —PSScriptAnalyzerinstall is unpinned while being tracked at1.25.0intool-checksums.json, creating an inconsistency between what is monitored and what is enforced.
There was a problem hiding this comment.
Advisory review — this PR is from a maintainer. Findings are informational only.
Review Summary
feat(security): add PSGallery staleness check and BOM cleanup
This is a well-structured, purposeful PR. The new Get-PSModuleStaleness function is clean, the OData response parsing handles both single- and list-result shapes, error paths are captured and returned rather than swallowed, and the 63-test coverage is thorough. The PowerShell-Yaml version pinning is consistent across all nine workflow files and both #Requires directives. Good work overall.
🔍 Issue Alignment
No GitHub issue is linked. The PR references PR #1378 as the source of the deferred advisories it addresses, which provides reasonable traceability, but the PR template requires a valid Fixes #N / Closes #N / Resolves #N reference in the Related Issue(s) section. Consider filing a tracking issue retroactively and linking it, or noting explicitly in the template that no issue exists per project policy.
📝 PR Template Compliance
The Related Issue(s) section contains "No tracking issue" rather than a valid issue reference — this is the only template gap. All type-of-change checkboxes are correctly checked and match the diff content. Testing evidence is present and specific.
Minor description inaccuracy: The description states the function "queries the PSGallery v2 OData API for every psModules entry in dependency-pinning.config.json", but the implementation reads from tool-checksums.json. The .SYNOPSIS inside the function itself is correct. The PR description should be updated to name the right file.
🛠️ Coding Standards
Two inline comments were left on scripts/security/Test-SHAStaleness.ps1:
- Line 737 —
Get-PSModuleStalenesscomment-based help is missing the required.EXAMPLEand.NOTESsections (perpowershell-script.instructions.md). The companionGet-PSGalleryApiBaseadded in this same PR is fully documented; applying the same pattern here keeps the module consistent. - Line 776 — The OData
$filterstring is built via direct interpolation of$mod.namewithout quote-escaping. Low risk today (static config), but worth a one-liner-replace "'", "''"` for robustness.
✅ Code Quality
- Response-structure fallback (
$response.propertiesvs$response.entry) is well-reasoned and the live-validation note in Additional Notes is appreciated. @(...)array-coercion guards on$moduleResults,$staleModules, etc. are correct and consistent with the existing script patterns.$script:StaleDependencies.Add(...)is the right pattern given the surrounding orchestrator structure.- BOM cleanup diff in
Test-CopyrightHeaders.Tests.ps1is a no-op at the text level — intentional and correct. - The forward-looking
sha256ByArch/assetTemplateByArchfields intool-checksums.jsonare cleanly scoped to thetoolsarray; the PR description's mention of "per-modulesha256ByArchfield" appears to refer to those tool entries, not to the PS module results. This could be phrased more clearly.
✅ Security
- No secrets or tokens introduced.
Get-PSGalleryApiBasecorrectly honoursHVE_PSGALLERY_REPOSITORYfor offline mirrors and test doubles, mirroringGet-GitHubApiBase.- The OData injection note (inline comment #2) is low severity given the current static config, but is worth addressing.
Action items:
- (Optional) Link or create a tracking issue and update the Related Issue(s) section.
- (Optional) Correct
dependency-pinning.config.json→tool-checksums.jsonin the PR description. - (Recommended) Add
.EXAMPLEand.NOTEStoGet-PSModuleStalenesscomment-based help. - (Recommended) Escape single quotes in the OData filter string.
- escape single quotes in OData filter for module name - pin PSScriptAnalyzer to 1.25.0 in copilot-setup-steps - add .EXAMPLE and .NOTES to Get-PSModuleStaleness help 🔒 - Generated by Copilot
There was a problem hiding this comment.
Review Summary
This PR ships a well-designed PSGallery staleness validator, consistent PowerShell module version pinning across all CI workflows, and BOM cleanup — solid work overall. The implementation follows project PowerShell conventions, the new Get-PSGalleryApiBase helper mirrors Get-GitHubApiBase correctly, and the Invoke-SHAStalenessCheck integration is well-structured.
🔗 Issue Alignment
Required fix: The Related Issue(s) section reads "No tracking issue." A linked GitHub issue is required for all PRs per project policy — even for follow-up work originating from a prior PR review. Please create a tracking issue (it can be minimal) and reference it with Fixes # or Closes #.
📋 PR Template Compliance
The Required Automated Checks section lists only three items (lint:ps, lint:md, lint:frontmatter). The current template requires eight. Several are N/A for this change type (validate:skills, plugin:generate), but the following should be explicitly acknowledged:
spell-check(npm run spell-check) — new docstrings and notes prose in.ps1and.jsonfileslint:md-links(npm run lint:md-links) — applicable to any markdown-adjacent changes
Please either check them (after running) or mark them N/A with a brief note.
🔍 Coding Standards
All changed PowerShell files follow the project conventions: copyright headers ✅, [CmdletBinding()] / [OutputType()] ✅, $ErrorActionPreference = 'Stop' in scope ✅, comment-based help ✅, module export updated ✅.
The workflow changes are precise and correct — replacing unversioned or conditionally-installed module calls with deterministic -RequiredVersion pins, and removing the now-redundant if (-not (Get-Module -ListAvailable ...)) guards that were unnecessary once versions are pinned.
💡 Code Quality
Two non-blocking observations are left as inline comments:
-
$response.entryfallback (line 794 ofTest-SHAStaleness.ps1) — The branch is confirmed dead code against PSGallery v2; a one-line comment explaining the intent (or removing it) would prevent future confusion. -
Missing test coverage for malformed
psModulesentries (Test-SHAStaleness.Tests.ps1line 300) — The production code path that handles entries with null/whitespacenameorversionfields is untested. Suggested test cases are provided in the inline comment.
✅ Security
HVE_PSGALLERY_REPOSITORYenv-var override for offline mirrors is a clean pattern that mirrorsHVE_PSGALLERY_SOURCE_URL✅- Single-quote escaping of module names prevents OData injection ✅
sha256ByArchpopulation with forward-compatible per-architecture checksums is well-documented ✅
Action Items
- Required: Add or link a GitHub issue and reference it in Related Issue(s).
- Required: Acknowledge
spell-checkandlint:md-linksin the Required Automated Checks section. - Suggested: Add a comment to the
$response.entryfallback or remove it. - Suggested: Add unit tests for malformed
psModulesentries (sample code in inline comment).
katriendg
left a comment
katriendg
left a comment
There was a problem hiding this comment.
Believe this looks great, I see two minor comments. Either keep as backlog/nice to have or fix and then merge.
- clarify retained Atom feed fallback branch with explanatory comment - add malformed psModules entry tests for missing name and version fields 🔒 - Generated by Copilot
There was a problem hiding this comment.
Advisory review, this PR is from a maintainer. Findings are informational only.
Overview
The PSGallery staleness validator implementation is clean and production-ready. Get-PSModuleStaleness is well-structured with consistent error-result objects, good OData escaping, and appropriate coverage of both the primary XmlElement response shape and the Atom feed fallback. The HVE_PSGALLERY_REPOSITORY override for offline test doubles mirrors the established Get-GitHubApiBase pattern correctly. The #Requires -Modules @{ RequiredVersion='0.4.7' } tightening and BOM cleanup are sound housekeeping. This review notes two template observations and confirms the previously-flagged coding issues are resolved in the current HEAD.
✅ Previously Flagged Items — Resolved
Both items from prior automated reviews are present in the current diff:
[OutputType([PSCustomObject[]])]is declared onGet-PSModuleStaleness(Test-SHAStaleness.ps1line ~738). ✅-Tag 'Unit'is present onDescribe 'Get-PSModuleStaleness'(Test-SHAStaleness.Tests.ps1). ✅
⚠️ Issue Alignment
No Fixes #, Closes #, or Resolves # reference is present. The PR description acknowledges this ("No tracking issue") and attributes the work to deferred advisories from #1378. The absence of a linked issue prevents GitHub from updating project boards or auto-closing work items on merge. If a lightweight tracking issue is not warranted here, consider using Related to #1378 in the Related Issue(s) section so the cross-reference link is at least rendered by GitHub.
⚠️ PR Template Compliance
Two gaps remain in the description relative to .github/PULL_REQUEST_TEMPLATE.md:
-
"GitHub Actions workflow" checkbox not checked. Eight workflow files were modified (all
Install-Moduleversion pins and conditional-install simplifications). TheInfrastructure & Configuration › GitHub Actions workflowbox should be checked to give reviewers an accurate picture of scope. -
Required Automated Checks list is partial. The template's automated checks checklist includes
npm run spell-check,npm run validate:skills,npm run lint:md-links,npm run plugin:generate, andnpm run docs:test. These are absent from the PR's checklist. For a PowerShell-only change, most are N/A — please check them and annotate each with(N/A — no skill/plugin/doc changes)so reviewers do not have to infer their status.
✅ Code Quality
- Error handling is thorough: every code path through
Get-PSModuleStalenessreturns a structured[PSCustomObject]with a populatedErrorfield rather than throwing, which integrates correctly with the caller's collection logic. - The
$malformedModulesvs$errorModulesdistinction inInvoke-SHAStalenessCheckcorrectly separates manifest validation failures (elevated toHighseverity) from network-level check failures (Medium). Compare-ToolVersionreuse for module version comparison is the right call — version normalisation logic stays in one place.- The
gitleaksnaming note ("Vendor names linux amd64 asset 'linux_x64'") is a helpful documentation addition to thesha256ByArchentry. - Test coverage at 86.81% (12 uncovered lines) is appropriate for a first-pass implementation; the uncovered lines appear to be in the Atom feed fallback path, which is intentionally guarded against live-API breakage.
nguyena2
left a comment
nguyena2
left a comment
There was a problem hiding this comment.
782 / 850 — Very Good 🟢
…ow version comments (#1389) # Pull Request ## Description Combines two related security/CI hygiene improvements into a single PR: 1. **Pin PSScriptAnalyzer to 1.25.0** at all install sites to ensure deterministic linting builds and address the supply-chain risk of pulling whatever version `Install-Module` resolves at run time. 2. **Sync stale GitHub Actions version annotation comments** across 39 workflow files so the trailing `# vX` comments accurately describe the SHA-pinned action versions. The SHA pins themselves are unchanged; only the human-readable annotations were drifting from reality, which `Test-ActionVersionConsistency` (`npm run lint:version-consistency`) was reporting as mismatches. ## Related Issue(s) Closes #1381 Refs #1379 ## Type of Change **Code & Documentation:** * [x] Bug fix (non-breaking change fixing an issue) **Infrastructure & Configuration:** * [x] GitHub Actions workflow * [x] Linting configuration (markdown, PowerShell, etc.) * [x] Security configuration * [x] DevContainer configuration **Other:** * [x] Script/automation (`.ps1`, `.sh`, `.py`) ## Testing * `npm run lint:version-consistency` ΓÇö `MismatchCount: 0` after annotation sync (was non-zero before). * SHA-to-tag mappings independently verified against the upstream GitHub API: * `actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd` ΓåÆ `v6.0.2` * `actions/upload-artifact@bbbca2ddaa5d8feaa63e36b76fdaad77386f024f` ΓåÆ `v7.0.0` * PSScriptAnalyzer pinning verified by inspecting the three install sites (`copilot-setup-steps.yml`, `on-create.sh`, `Invoke-PSScriptAnalyzer.ps1`) ΓÇö all now request `-RequiredVersion 1.25.0`. ## Checklist ### Required Checks * [x] Files follow existing naming conventions * [x] Changes are backwards compatible (annotation-only edits + version-pinning) ### Required Automated Checks * [x] PowerShell analysis: `npm run lint:ps` * [x] Action version consistency: `npm run lint:version-consistency` * [x] Markdown lint: `npm run lint:md` ΓÇö N/A (no markdown content changes; only YAML comments and PowerShell version strings) * [x] Spell check: `npm run spell-check` ΓÇö N/A (no prose content changes) * [x] Frontmatter: `npm run lint:frontmatter` ΓÇö N/A (no frontmatter touched) * [x] Skill validation: `npm run validate:skills` ΓÇö N/A (no skills modified) * [x] Markdown links: `npm run lint:md-links` ΓÇö N/A (no markdown changes) * [x] Plugin generate: `npm run plugin:generate` ΓÇö N/A (no collections, agents, prompts, instructions, or skills modified) * [x] Docs test: `npm run docs:test` ΓÇö N/A (no docs content changes) ## Security Considerations * [x] This PR does not contain any sensitive or NDA information * [x] Pinning PSScriptAnalyzer reduces install-time supply-chain variance * [x] No changes to action SHA pins; only the annotation comments were corrected ## Additional Notes Scope note: in addition to the two annotations called out by the validator (`# v4.4.3` ΓåÆ `# v7`, `# v4.2.2` ΓåÆ `# v6.0.2`), two further annotations on the same SHAs were drifting (`# v6` ΓåÆ `# v7`, `# v6.0.1` ΓåÆ `# v6.0.2`) and were corrected in the same pass for consistency. All edits are comment-only ΓÇö no SHA, no behavior change. `copilot-setup-steps.yml` also pins `PowerShell-Yaml -RequiredVersion 0.4.7` alongside the PSScriptAnalyzer pin, matching the `#Requires -Modules PowerShell-Yaml, RequiredVersion='0.4.7'` directives introduced in #1379. A trailing newline was added to the same file; harmless and conforms to POSIX text-file convention. ## Merge Update Merged `origin/main` (commit `3b3b8153`). 27 workflow files had concurrent SHA-pin updates on main; resolved by taking main's newer SHAs since they already carried the corrected version annotations this PR was introducing. Net effect: PR's annotation-correction goal is satisfied; main's freshness is preserved. Verified `copilot-setup-steps.yml` retains both `-RequiredVersion` pins (PSScriptAnalyzer 1.25.0, PowerShell-Yaml 0.4.7). Post-merge validation: `lint:yaml` (48 files), `lint:version-consistency` (361 actions, 0 mismatches), `lint:permissions` (47/47) all pass.
## Pre-Release 3.3.101 ### ✨ Features - add removed maturity tier and retire owasp-docker (#1444) - add evaluation dataset creator (#1279) - align RAI planner with guide, remove scoring, improve UX (#1287) - add PSGallery staleness check and BOM cleanup (#1379) - ISA-95 network planner agent (#1177) - auto-generate collection.md with maturity filtering (#1316) - add folder-consistency check and standardize WARN outp… (#1350) - add synth-data-generate prompt to data-science collection (#1419) - add canonical deck workflow and customer-card rendering for design thinking (#1413) - add Figma MCP integration for DT artifact export (#1222) - introduce `owasp-docker` (#1245) - replace hve-core-specific references with portable discovery-based language (#1335) - introduce `owasp-cicd` (#1246) - add secure-by-design knowledge skill (#1223) - introduce `owasp-infrastructure` (#1244) - introduce `owasp-mcp` (#1207) - add OutputPath parameter to Invoke-LinkLanguageCheck.ps1 (#1229) - add -OutputPath parameter to Validate-SkillStructure.ps1 (#1225) - add maintainer-only skip-review label guard (#1293) - add extension collections overview and integrate into getting started flow (#950) - add agentic workflows for automated issue triage, implementation, PR review, dependency review, and doc-staleness detection (#1219) - consolidate package-lock.json version sync into Update-VersionFiles.ps1 (#1240) - add standards code review agent and full review orchestrator (#1174) - standardize pytest-mock as Python mocking framework (#1170) - add Jira backlog workflows and Jira/GitLab skills (#978) - add centralized version bump script and supply-chain attestation (#1183) ### 🐛 Bug Fixes - pin PowerShell-Yaml to 0.4.7 across all install sites (#1378) - close fork-PR/workflow-file-PR secret-strip gap and normalize upload-artifact version (#1421) - replace stream-based lookahead with array indexing in list-changed-files.sh (#1376) - centralize ISO 8601 timestamp regex in CIHelpers (#1343) - update stale documentation date in release-process.md (#1363) - pin basic-ftp to 5.3.0 to resolve GHSA-rp42-5vxx-qpwr (#1374) - add bot filter to dependency PR review workflow (#1362) - resolve pip-audit findings in powerpoint, gitlab, and jira skill lock files (#1360) - standardize Timestamp JSON key casing across all lint result files (#1314) - add synchronize trigger to PR Review workflow (#1323) - standardize timestamp in Validate-SkillStructure.ps1 to use Get-StandardTimestamp (#1280) - add parallel subagent dispatch and structured JSON contracts to code-review-full (#1304) - standardize timestamp in SecurityHelpers.psm1 to use Get-StandardTimestamp (#1284) - standardize timestamps in Test-DependencyPinning.ps1 and SecurityClasses.psm1 (#1282) - derive collection artifact counts from YAML at build time (#1275) - standardize timestamp in FrontmatterValidation.psm1 to use Get-StandardTimestamp (#1285) - standardize timestamp in Markdown-Link-Check.ps1 to use Get-StandardTimestamp (#1283) - escape hyphens in Mermaid diagram on Collections page (#1262) - add summary timestamp to PSScriptAnalyzer output (#1211) - fix plugin compatibility and robustness for coding-standards code review agents (#1289) - standardize timestamp in Test-CopyrightHeaders.ps1 to use Get-StandardTimestamp (#1278) - standardize timestamp in Invoke-YamlLint.ps1 to use Get-StandardTimestamp (#1270) - standardize timestamp in Invoke-LinkLanguageCheck.ps1 to use Get-StandardTimestamp (#1264) - fix dependency-review path filters and sparse-checkout cone mode (#1259) - replace invalid bare tool names with official tool identifiers (#1198) - fix broken links and remove orphaned reference in code review docs (#1257) - exclude Python env dirs from skill validation warnings (#1255) - pin happy-dom and serialize-javascript to resolve Dependabot vulnerabilities (#1253) - remove Mermaid diagram and add missing collection cards (#1247) - disable MCP servers by default to prevent token limit errors (#1144) - sync package-lock.json after pre-release version bump (#1236) - separate mermaid node declarations and add dynamic diagram generation with tests (#1215) - replace anchor links in meeting-analyst with bold text references (#1201) - remove recursive symlinks in jira and gitlab skill directories (#1233) - validate-installation scripts now check .github/skills directory (#1010) (#1206) - resolve npm audit vulnerabilities via dependency overrides (#1200) - add post-release triggers to scorecard workflow (#1186) - add missing .md extensions to relative links in agent documentation (#1180) ### 📚 Documentation - broaden Security Review description beyond OWASP (#1385) - document maintainer advisory mode and skip-review label guard (#1386) - document ExcludePaths/OutputPath for Invoke-LinkLanguageCheck (#1383) - CLI getting-started: clarify plugin install commands as alternatives (-all vs base) (#1251) ### ♻️ Refactoring - align agent and prompt folder names to collection identifier (#1210) ### 🔧 Maintenance - pin PSScriptAnalyzer to 1.25.0 and sync stale workflow version comments (#1389) - bump lxml from 6.0.2 to 6.1.0 in /.github/skills/experimental/powerpoint (#1424) - bump @vscode/vsce from 3.7.1 to 3.9.1 in the npm-dependencies group (#1390) - bump the github-actions group across 1 directory with 7 updates (#1391) - bump follow-redirects from 1.15.11 to 1.16.0 in /docs/docusaurus (#1356) - upgrade Node.js from 20 to 24 and bump cspell to v10 (#1353) - bump basic-ftp from 5.2.0 to 5.2.1 (#1324) - update github/gh-aw-actions requirement to 536ea1bad8c6715d098a9dc1afea8d403733acfe in the github-actions group across 1 directory (#1298) - update security instruction attributions and compliance (#1294) - bump the npm-dependencies group with 2 updates (#1297) - pre-release 3.3.41 (#1252) - streamline RAI Planner phase structure and documentation (#1273) - bump happy-dom from 20.8.8 to 20.8.9 in /docs/docusaurus (#1237) - pre-release 3.3.27 (#1191) - bump pygments from 2.19.2 to 2.20.0 in /.github/skills/gitlab/gitlab (#1234) - bump path-to-regexp from 0.1.12 to 0.1.13 in /docs/docusaurus (#1226) - bump the github-actions group with 4 updates (#1231) - add missing folders and alphabetize location lists (#1193) - bump brace-expansion (#1224) - bump handlebars from 4.7.8 to 4.7.9 in /docs/docusaurus (#1217) - bump brace-expansion from 5.0.3 to 5.0.5 in /docs/docusaurus (#1213) - pre-release 3.3.10 (#1187) - bump markdownlint-cli2 from 0.21.0 to 0.22.0 in the npm-dependencies group (#1175) - bump the github-actions group with 3 updates (#1176) - pre-release 3.3.1 (#1165) --- *Managed automatically by pre-release workflow.* Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
4 participants
Description
Adds a PowerShell Gallery staleness validator to
Test-SHAStaleness.ps1and removes a stray UTF-8 BOM from collection-helper modules.The new
Get-PSModuleStalenessfunction queries the PSGallery v2 OData API for everypsModulesentry independency-pinning.config.jsonand returns whether each pinned version matches the latest published version. The orchestrator (Test-SHAStaleness) now reports stale PowerShell modules alongside GitHub Actions, including a per-modulesha256ByArchfield for forward-compatible per-architecture checksum tracking.The BOM cleanup pass strips the leading
EF BB BFbyte sequence fromCollectionHelpers.psm1andFind-CollectionManifests.ps1. While editing those files, the existing#Requires -Modules PowerShell-Yamldirectives were tightened to pinRequiredVersion='0.4.7'to match the version installed bycopilot-setup-steps.ymland the devcontainer.Related Issue(s)
No tracking issue. Implements the deferred review advisories from PR #1378 (per-architecture checksum field + PSGallery staleness check) and resolves BOM lint findings surfaced during that review.
Type of Change
Code & Documentation
Infrastructure & Configuration
Testing
npm run test:ps -- -TestPath scripts/tests/security/Test-SHAStaleness.Tests.ps1— 63/63 tests pass.Get-PSModuleStalenessagainst PSGallery returned the expected stale/current verdicts for every pinned module independency-pinning.config.json.npm run lint:ps— clean.Checklist
Required Checks
Required Automated Checks
lint:pspasses locallylint:mdpasses locallylint:frontmatterpasses locallySecurity Considerations
Additional Notes
Out-of-scope changes documented for reviewers:
CollectionHelpers.psm1andFind-CollectionManifests.ps1had their#Requires -Modules PowerShell-Yamldirectives upgraded to pinRequiredVersion='0.4.7'. This is a behavioral tightening (strict version match atImport-Moduletime) made while touching the files for BOM cleanup; the pinned version matches the version provisioned bycopilot-setup-steps.ymland the devcontainer setup.sha256ByArchfield on PowerShell-module results is forward-looking infrastructure. It is populated but not yet consumed by downstream tooling; consumers will be added in a follow-up PR once the multi-architecture checksum source is finalized.PSGallery OData response structureconcern raised by the automated reviewer was investigated against a live PSGallery response:Invoke-RestMethodreturns the<entry>XmlElementdirectly for single-package queries, so$response.properties.Versionis the correct access path. End-to-end runs ofGet-PSModuleStalenessconfirm correct stale/current detection. No code change required.