[polewskm]

Why

CodeQL's extended query suite, now on for every NCodeGroup repository, reports a third-party action referenced by a tag as actions/unpinned-tag: whoever controls that repository can move the tag, and the step runs with this workflow's token and secrets. A full commit SHA is the one reference nobody can move.

Changes

Third-party actions are pinned to the commit SHA of the release their tag pointed at, with the release in a comment, so Dependabot's github-actions updates keep bumping them (it updates the SHA and the comment together). Behavior is unchanged: each SHA is the commit the tag resolves to today.

Verified

Each SHA was resolved from the tag with gh api repos/<action>/commits/<tag> and matched to the release tag with git ls-remote --tags. The CodeQL alerts for these lines close when this merges.

🤖 Generated with Claude Code

[ncode-review]

🤖 NCode Review at 2ae34a3: 2 new findings on the changed lines.

Not on a changed line:

• 🟠 F1 Should fix: Most actions in main.yml still use movable tags, and checkout keeps credentials .github/workflows/main.yml:26. Only two actions are pinned. actions/checkout@v7 (lines 26, 57, 85), actions/setup-dotnet@v6 (28, 63, 95), actions/upload-artifact@v7 (41) and actions/download-artifact@v8 (59, 87) still use movable tags, and no checkout sets persist-credentials: false. Fix: Pin the remaining actions to full SHAs with # vX.Y.Z comments, at minimum in the publish job, and add with: persist-credentials: false to each checkout step. Otherwise, note in the PR that this is left for a follow-up.
• 🟡 F4 Nit: Publish step uses a stored NuGet key and skips several safeguards (existing issue) .github/workflows/main.yml:99. The publish step pushes with secrets.NUGET_APIKEY placed directly into the shell command. It has no --skip-duplicate, never checks that the package version matches the tag, and doesn't push the .snupkg files that the earlier gh release upload step uploads. The job also has no environment: gate. Fix: In a follow-up: switch to NuGet trusted publishing (OIDC), add --skip-duplicate, push the .snupkg files, check the tag against the package version, and add an environment: to the publish job.
• 🟡 F5 Nit: Release tag_name is placed directly into a shell script (existing issue) .github/workflows/main.yml:91. ${{ github.event.release.tag_name }} is expanded inline in a run script in the job that holds contents: write and the NuGet key. Fix: Pass the value through env: TAG_NAME: ${{ github.event.release.tag_name }} and reference it as "$TAG_NAME" in the script.

[ncode-review]

🟡 Nit: Check that the dorny/test-reporter SHA matches v3.2.0 and is the latest v3 release _{F2 · rule-check}

The pin moves from floating v3 to a SHA labelled v3.2.0, and no reviewer confirmed that the SHA matches that tag or that it is the newest v3.x release.

Consequence: If they don't match, the pin is stale or the comment is misleading, and this step runs with checks: write.

Fix: Check that d1b5dff021aa968b288b02285a8bf47471c5928a is the commit for upstream tag v3.2.0, and that v3.2.0 is the latest v3 release.

Evidence

Rule: pin the latest release from upstream when adding or bumping a pin.

Confidence 0.30.

[ncode-review]

🟡 Nit: Should dorny/test-reporter be replaced with a job summary instead of being pinned? _{F3 · rule-check}

dorny/test-reporter creates check runs and needs checks: write (granted at line 55). The repo rule says to report results through $GITHUB_STEP_SUMMARY and ::error annotations instead. This predates the PR, but pinning the action locks it in.

Consequence: On PRs from forks the token has no checks: write. With if: success() || failure(), the reporting step can then fail and add a failing step to the run.

Fix: Consider a follow-up that removes dorny/test-reporter and checks: write, writes the TRX results to $GITHUB_STEP_SUMMARY or as ::error annotations, and leaves the dotnet test exit code as the only gate.

Evidence

Rule: present CI results through job summaries and annotations, not check-run actions that need checks: write.

Confidence 0.50.

[ncode-review]

🤖 Review complete

Verdict: 💬 Comment — This PR pins two third-party actions in .github/workflows/main.yml, dhkatz/get-version-action and dorny/test-reporter, to commit SHAs with version comments. The change is small and low-risk, but it only covers part of the file. Checkout, setup-dotnet, upload-artifact and download-artifact still use movable tags, including in the publish job that holds the NuGet key, while ci.yml and code-review.yml already pin theirs. The other findings are older issues in the same workflow, raised as questions for a follow-up.

4 reviewers run — 5 findings (run details)

Reviewer	Model	Findings	Top
synthesis	opus-5-5	5	🟠
conventions	sonnet-5-5	0	🟢
hygiene	haiku-5-5	1	🟠
rule-check	sonnet-5-5	4	🟠
security	sonnet-5-5	2	🟠

Skipped: api-compat, architecture, completeness, correctness, data-model, infrastructure, intent, performance, tests

Findings

• 🟠 F1 .github/workflows/main.yml:26: Most actions in main.yml still use movable tags, and checkout keeps credentials
• 🟡 F2 .github/workflows/main.yml:71: Check that the dorny/test-reporter SHA matches v3.2.0 and is the latest v3 release
• 🟡 F3 .github/workflows/main.yml:71: Should dorny/test-reporter be replaced with a job summary instead of being pinned?
• 🟡 F4 .github/workflows/main.yml:99: Publish step uses a stored NuGet key and skips several safeguards (existing issue)
• 🟡 F5 .github/workflows/main.yml:91: Release tag_name is placed directly into a shell script (existing issue)

_{Trial: findings are posted as review threads and in the check run. $0.23 on Amazon Bedrock, 53s, at 2ae34a3. (how to interact)}