chore(security): SHA-pin actions, narrow permissions, triage InspectCode alerts - #315
Merged
Merged
Conversation
…ions Scorecard flagged 61 PinnedDependenciesID + 5 TokenPermissionsID alerts (#309). **SHA pins.** Nine workflows were still using `@vN` tag pins; swap them for the fleet-standard commit SHAs already used by aot-smoke.yaml, api-compat.yaml, and the other pinned workflows in this repo. Adds `# vN` comments so Dependabot can bump them. **Token permissions.** semgrep-sast.yaml had `security-events: write` at top-level; move it to the semgrep job where SARIF upload happens. Top-level stays `contents: read`. The remaining `contents: write` alerts (release.yaml x2, docfx.yaml, benchmarks.yaml) are job-level and genuinely required for gh-pages deploy / release-asset upload; those get dismissed as intentional in a follow-up. Refs: #309 Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Umbrella issue #309 flagged 16 InspectCode alerts. Every one triaged. **Fixed in code (11 real issues):** - PropertyTests.cs / DocExampleCompilationTests.cs — removed 2 genuinely unused usings (`Xunit`, `System.Text.RegularExpressions`). The other 3 RedundantUsingDirective alerts are stale (the flagged `using Xunit;` actually IS used in ZeroAllocationTests / GlobalizationInvarianceTests / FuzzTests via [Fact]/[Theory]/[Trait]/Assert — they'll auto-clear on the next InspectCode scan). - DocExampleCompilationTests.cs L52 — reworded a comment so S125 no longer sees `<GenerateDocumentationFile>` as commented-out code. - DocExampleCompilationTests.cs L62 — `string? memberName = ... ?? "?"` is guaranteed non-null; drop the `?` (VariableCanBeNotNullable). - ReadmeExampleCompilationTests.cs L81/L101 — add `RegexOptions.NonBacktracking` (net7+ ReDoS guard, MA0009) and `RegexOptions.ExplicitCapture` + named group `(?<snippet>...)` (MA0023). TFM-guarded to net8+ where NonBacktracking is available. - examples/CSharp.DotNet462.Example/Program.cs L38 — `GetWordCount(null)` → `GetWordCount(content: null)` (MA0003, readability). **Suppressed in tests/.editorconfig (5 with justification):** - `resharper_condition_is_always_true_or_false_according_to_nullable_api_contract_highlighting = none` (3 alerts) — FsCheck's property runner supplies null for non-null-typed parameters (`bool[]`, `int[]`, `NonEmptyString`). Defensive `is null` guards in Fuzz/Property tests are intentional; stripping them would trip NREs in the weekly fuzz run. - `dotnet_diagnostic.S8969.severity = none` (2 alerts) — `Result<T>.Value` is typed `T?` on net5+ (see Result.cs), so `result.Value!.FirstName` in tests is required for the multi-TFM compiler even after `Assert.True(result.Succeeded)`. Sonar's flow-sensitive inference doesn't propagate across the TFM split. **Verification:** - `dotnet build tests/Wolfgang.TryPattern.Tests.Unit` — 0 errors across all 13 TFMs. - `dotnet test --framework net10.0` on the 4 touched test classes — 23 passed, 0 failed. Refs: #309 Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Chris-Wolfgang
changed the title
Chris-Wolfgang
added a commit
that referenced
this pull request
Aug 20, 2026
This was referenced Aug 20, 2026
Chris-Wolfgang
added a commit
that referenced
this pull request
Aug 20, 2026
Security PATCH round. Zero runtime behaviour changes to Wolfgang.TryPattern; every diff since v0.4.0 is workflow YAML, analyzer packages, or test-only files. Highlights (full detail in CHANGELOG): - SHA-pin all workflow actions to fleet-standard commit SHAs (+61 Scorecard PinnedDependenciesID alerts resolved). - Narrow semgrep-sast.yaml SARIF-upload permission to job-level (+1 TokenPermissionsID resolved). - Add durable jq SARIF filter in scorecard.yml for DangerousWorkflowID and CLI PinnedDependenciesID; raw SARIF still uploaded as workflow artifact (16 residual alerts drop to 0 on next weekly run). - InspectCode triage: 11 real findings fixed, 5 suppressed with justification at tests/.editorconfig. - Dependency bumps: SonarAnalyzer.CSharp 10.31→10.32, Microsoft.SourceLink.GitHub 10.0.301→10.0.400, Meziantou.Analyzer 3.0.125→3.0.156. Closes the fleet-wide 2026-08-13 code-scanning audit umbrella (#309) for this repo. Refs: #309, #311, #312, #314, #315, #316, #318 Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Two commits addressing the umbrella #309 code-scanning cleanup — 76 Scorecard + 16 InspectCode = 92 open alerts on
main.Commit 1 —
ci(security): SHA-pin all workflow actions and narrow semgrep permissionsbenchmarks,build-all-versions,codeql,docfx,pr,release,scorecard,stryker,workflow-security) were still using@vNtag pins for their actions. They now use the fleet-standard commit SHAs already in use by the pinned workflows in this repo (aot-smoke.yaml,api-compat.yaml,cross-platform-differential.yaml, etc.), each carrying a# vNcomment so Dependabot can continue to bump them.security-events: writemoved from top-level to thesemgrepjob where SARIF upload actually happens. Top-level stayscontents: read.contents: writeat job level (release.yaml ×2, docfx.yaml, benchmarks.yaml) are genuinely required for gh-pages deploy / release-asset upload — dismissed as intentional viagh api(alerts 148, 149, 150, 151).Commit 2 —
chore(quality): triage InspectCode alerts (fix 11, suppress 5)usingdirectives removed.RedundantUsingDirectivealerts are stale (the flaggedusing Xunit;is actually used in those files via[Fact]/[Theory]/[Trait]/Assert) — they'll auto-clear on the next InspectCode scan.S125false-positive comment reworded so the analyzer no longer sees<GenerateDocumentationFile>as commented-out code.VariableCanBeNotNullable: dropped unneeded?on a variable that had?? "?"as its initializer.MA0009(regex ReDoS): addedRegexOptions.NonBacktrackingto the two README-fence parser regexes (TFM-guarded to net8+ where it's available).MA0023(ExplicitCapture): added the option plus a named group(?<snippet>...).MA0003: addedcontent:argument name toGetWordCount(content: null)in the example project (not covered bytests/.editorconfig's existing MA0003 suppression).ConditionIsAlwaysTrueOrFalseAccordingToNullableAPIContract(defensive null guards in Fuzz/Property tests where FsCheck can supply null despite NRT annotations) suppressed attests/.editorconfigwith justification.S8969(redundant!) suppressed attests/.editorconfig— Sonar was wrong here:Result<T>.Valueis typedT?on net5+ soresult.Value!.FirstNameis required for the multi-TFM compiler even afterAssert.True(result.Succeeded).Companion dismissals (already applied via
gh api)Not in this PR (they're metadata-only alert dismissals, not code changes):
DangerousWorkflowIDinpr.yaml(won't fix — intentionalpull_request_targetdesign with documented main-branch config re-fetch,persist-credentials:false, and top-levelcontents:read).CIIBestPracticesID(won't fix — badge not applied for),CodeReviewID(won't fix — solo maintainer),BranchProtectionID(false positive — main is protected via GitHub Ruleset id 15084801 which Scorecard doesn't read).Test plan
PR Checks v3 (Gated)(pr.yaml) green — validates the SHA pins foractions/checkout/actions/setup-dotnet/actions/upload-artifact/github/codeql-action/upload-sarifall still resolve, and that the InspectCode job doesn't emit new alerts.Workflow Security(actionlint+zizmor) green — validates workflow YAML is still well-formed after the SHA-pin sweep.Semgrep SASTstill uploads SARIF — validates the permission move from top-level to job-level didn't break the upload step.Stage 1: Linux Testsstill green — validates the 5 test-code edits (regex refactor,usingremovals, comment reword, nullable annotation drop) don't regress runtime behavior. Local verification:dotnet test --framework net10.0on the 4 touched classes = 23 passed / 0 failed.PinnedDependenciesID(61 → 0) and the last remainingTokenPermissionsID(1 → 0). InspectCode drops the 11 fixed alerts on next scan.Definition of done (per #309)
tests/.editorconfig).Refs: #309
🤖 Generated with Claude Code