fix(scripts): align Grype writer/reader naming so security gate fails closed (#362)

[WilliamBerryiii]

Summary

Fixes #362. The Grype writer emitted timestamped report basenames while the security gate reader globbed an incompatible pattern, so zero matches were silently treated as a pass — a false-negative security gate.

Changes

• Writer (scripts/security/Invoke-ContainerSecurityScan.ps1): sanitize image/tag ([\/:] → _) in Grype report basename: grype-${safeImage}-${safeTag}-${timestamp}.json.
• Reader (scripts/security/Invoke-SecurityGate.ps1): recurse with grype-*.json glob and throw on zero reports (fail-closed). Same recursive pattern applied to Checkov/DependencyAudit (warn-only). Fix typo $gryteFindings → $grypeFindings.
• Builder (scripts/build/modules/ApplicationBuilder.Build.psm1): route container scan output to per-service ./security-reports/<service> path.
• Pipeline (.azdo/templates/application-build-template.yml): pass -ExitOnFailure through to gate invocation.
• Tests: add Pester v5 unit tests for writer basename sanitization and reader fail-closed behavior (6/6 green).

Validation

• Invoke-Pester — 6/6 tests pass in 2.41s
• Invoke-ScriptAnalyzer — clean across all touched files

Follow-up (not in this PR)

• test-results/ is not in .gitignore; recommend adding in a separate hygiene PR.

🔒 - Generated by Copilot

[katriendg]

Thorough review — nice, tightly-scoped fix overall. One likely functional regression to address (the ADO $${{ ... }} escape), one indent cleanup, and two non-blocking notes. Details inline.

Items flagged:

• 🔴 Major: -ExitOnFailure $${{ parameters.breakBuild }} in application-build-template.yml — wrong escape form; will render literally and break the gate step.
• 🟡 Minor: Get-CheckovScanResult inner try/catch left over-indented after the outer loop was removed.
• 🟢 Nit: symmetry — sanitize $service when composing the per-service report path in ApplicationBuilder.Build.psm1.
• 🟢 Info: tests rely on AST-based function harvesting; consider a Main guard in the SUT as a future cleanup.

Also — the PR body mentions test-results/ missing from .gitignore is deferred; could you link the tracking issue/PR so it doesn't fall through?

Looks good otherwise: Grype basename sanitization, recursive fail-closed reader, per-service report path to avoid concurrent-scan collisions, and the $gryteFindings typo fix are all on point and well-tested.

[katriendg]

Likely bug: wrong ADO expression-escape form (functional regression).

$${{ ... }} is the Azure Pipelines escape sequence that emits a literal ${{ ... }} after template processing (docs). So this line renders on the agent as:

-ExitOnFailure ${{ parameters.breakBuild }}

…which PowerShell receives verbatim and rejects as an invalid argument — causing the security-gate step itself to fail with a parse error, defeating the PR's stated goal of making the gate fail-closed on real findings only.

Additionally, parameters.breakBuild is a boolean that template-expands to True/False (Pascal-case), not $true/$false, so even a non-escaped expansion would not produce a valid PowerShell boolean literal.

Recommended fix — conditional insertion (idiomatic for this repo, no escaping needed):

          ${{ if eq(parameters.breakBuild, true) }}:
            -ExitOnFailure $true
          ${{ else }}:
            -ExitOnFailure $false

Or alternatively pass through a pipeline variable and convert in PowerShell ([System.Convert]::ToBoolean($env:EXITONFAILURE)), but the conditional form is cleaner.

[katriendg]

Nit: symmetry with the writer's sanitization.

$service is joined into a path here without sanitization. In practice service names come from repo config so risk is nil, but the companion writer in Invoke-ContainerSecurityScan.ps1 now sanitizes ImageName/ImageTag with [\\/:] → _ for exactly this reason. Keeping the two sides symmetric avoids surprises if a service name ever contains a separator:

$safeService = ($service -replace '[\\/:]', '_').Trim('_')
$serviceReportPath = Join-Path './security-reports' $safeService

Non-blocking.

[katriendg]

Maintainability note: AST-based SUT loader is fragile.

Both new test files use [System.Management.Automation.Language.Parser]::ParseFile to harvest only function definitions and skip the SUT's trailing Main call. This works today but will silently break on future refactors that introduce using statements, nested classes, or top-level state that the functions close over.

Non-blocking suggestion for a follow-up: guard the top-level call in the SUT with

if ($MyInvocation.InvocationName -ne '.') {
    Main @PSBoundParameters
}

so the test file can become a one-line . $script:SutPath. Applies equally to Invoke-ContainerSecurityScan.Tests.ps1.

[katriendg]

Stale inner indentation in Get-CheckovScanResult after loop-removal refactor.

When the outer foreach ($pathPattern in $checkovPaths) loop was dropped, the inner try { … } catch { … } body kept its old 12-space indent while the new surrounding foreach ($file in $checkovFiles) { is at 4 spaces. The result compiles but is visually misaligned and asymmetric with the sibling Get-GrypeScanResult / Get-DependencyAuditResult parsers in this same PR, which were re-indented correctly.

Please re-indent the try/catch body one level under the single foreach so all three parsers read consistently.