Skip to content

Pin compiler problem matcher Actions to commit SHAs - #3724

Merged
airween merged 4 commits into
owasp-modsecurity:v3/masterfrom
7acini:chore/pin-ci-problem-matchers
Oct 30, 2026
Merged

airween merged 4 commits into
owasp-modsecurity:v3/masterfrom
7acini:chore/pin-ci-problem-matchers

Conversation

@7acini

@7acini 7acini commented Oct 30, 2026 •

Copy link
Copy Markdown
Contributor

The QA workflows currently load the GCC and MSVC problem matchers from mutable
master branches. Pin these five references to full upstream commit SHAs so
workflow executions use a fixed revision of each third-party Action.

This change is isolated from log-message escaping, as requested in the review
of ModSecurity PR #3643. Job matrices and build/test behavior are preserved. A separate follow-up
commit replaces legacy backtick command substitution with make -j "$(nproc)"
in ci.yml, as requested for SonarCloud rule githubactions:S7689.
The revisions are the same ones previously included in that PR:

  • GCC: cb2e3f949c41b7818628f35f8484a9b0acf90cf9 — master (2026-03-12)
  • MSVC: 47189a176388253d16c6f0b79eee6274512b761a — master (2026-09-05)

Each pin has an inline revision/date comment matching the style of
release-windows.yml. The branch includes v3/master through ee0c377c;
that release workflow is unchanged.

Validation: both workflow files parse as YAML, all five matcher references use
full 40-character SHAs, and structural comparison against the updated base
confirms only the pins and command substitution differ. Bash syntax and the
resulting make arguments were checked without invoking a build.
git diff --check passes. Fresh remote CI results are still required.

Summary by CodeRabbit

  • Chores
    • Updated automated build checks across Linux, macOS, and Windows to use fixed versions of their diagnostic tools. This keeps the tools used to report build issues consistent across runs. The Linux build also retains its existing processor-count-based parallelism with updated command syntax; no change to the end-user experience is expected.

@coderabbitai

coderabbitai Bot commented Oct 30, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

No actionable comments were generated in the recent review.

Recent review info
Run configuration
  • Source mode: Auto
  • Repository: Unavailable
  • Branch: Unavailable
  • File: No configuration file from this repository applied
  • Commit: Unavailable
  • Layers: defaults
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 7c6e65b4-2caa-4085-b369-17f878297460
Commits

Reviewing files that changed from the base of the PR and between 3fe298a and 8416a7d.

Files selected for processing: 2
  • .github/workflows/ci.yml
  • .github/workflows/ci_new.yml

Included review availability: This review used your included allowance. Your plan provides up to 8 included reviews per hour; 7 remain after this review.

Walkthrough

The CI workflows now pin GCC and MSVC problem matcher actions to specific commit SHAs instead of the mutable master branch. The Linux make step uses $() command substitution for nproc.

Changes: CI matcher pins

Layer / File(s) Summary
Pin problem matcher references and update Linux command substitution
.github/workflows/ci.yml, .github/workflows/ci_new.yml
Linux, macOS, and Windows jobs now reference GCC and MSVC problem matchers by commit SHA. The Linux make step uses $() instead of backticks for nproc.

Priority: Low

Estimated code review effort: 1 (Trivial) | ~5 minutes

Change: Other

Possibly related PRs

Suggested reviewers: easton97-jens, eduar-hte

Merge Risk: Minimal · up to 8416a

The workflow changes are limited to verified immutable pins and an equivalent shell-syntax improvement, with no merge-blocking risk identified.

Pre-merge checks: Passed 4
Check name Status Explanation
Description Check Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check Passed The title clearly and concisely describes the primary change: pinning compiler problem matcher Actions to commit SHAs. It matches the changes in both workflow files.
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
Finishing Touches
🧪 Generate unit tests (beta)
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@airween

airween commented Oct 30, 2026

Copy link
Copy Markdown
Member

Hi @7acini, thanks for splitting this out of #3643. I checked both SHAs, and they are the current master HEADs of ammaraskar/gcc-problem-matcher and ammaraskar/msvc-problem-matcher. CI therefore runs exactly the same code as before, now pinned, and the full matrix is green.

A few things before we merge:

  1. Please update your branch. It is 25 commits behind v3/master. Among those commits is a new workflow, .github/workflows/release-windows.yml, which already pins all of its actions to SHAs, so it's worth checking your change against it once you've rebased (or merged v3/master in).

  2. Add a comment after each pinned SHA. release-windows.yml already uses this style, e.g.:

    - uses: ammaraskar/msvc-problem-matcher@47189a176388253d16c6f0b79eee6274512b761a # master (2026-09-05)

    Please do the same for the five references here, so it's clear what each SHA is when someone bumps it later. For the GCC matcher that would be # master (2026-03-12).

  3. SonarCloud issue. Sonar reports one new issue on this PR, githubactions:S7689 at .github/workflows/ci.yml:104: "Replace backticks with '$()' syntax for command substitution."

    run: make -j `nproc`

    I know that line isn't yours, since it's already like this on v3/master. But Sonar attributes it to this PR, and ci_new.yml already uses the correct form. Could you fix it in a separate commit in this PR?

    run: make -j "$(nproc)"

Thanks!

@7acini

7acini commented Oct 30, 2026

Copy link
Copy Markdown
Contributor Author

Thanks @airween. All three requested updates are now in this PR:

  1. Merged v3/master through ee0c377c in 71784591, bringing in the 25 upstream commits. I checked the new release-windows.yml; its existing MSVC SHA and inline comment match this PR, and that workflow remains unchanged.
  2. Added revision/date comments to all five pins in a0a6c0b5: # master (2026-03-12) for GCC and # master (2026-09-05) for MSVC.
  3. Fixed githubactions:S7689 in the separate commit 8416a7df, replacing the backtick substitution with make -j "$(nproc)" in ci.yml.

Validation: both YAML files parse successfully; structural comparison with the updated base confirms only the matcher pins and that command substitution differ. Job matrices are unchanged. Bash syntax, the resulting make arguments, and git diff --check passed. I did not rerun the C++ suite locally for these workflow-only changes.

The PR description is updated. Both fresh QA runs for 8416a7df currently need maintainer approval (action_required): Quality Assurance and Quality Assurance new. Please approve them so the updated head can be validated.

@sonarqubecloud

Copy link
Copy Markdown

@airween

airween commented Oct 30, 2026

Copy link
Copy Markdown
Member

Thanks @7acini, merging now.

@airween
airween merged commit 1bfaff3 into owasp-modsecurity:v3/master Oct 30, 2026
89 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants