Skip to content

SQSCANGHA-140 Set skipSignatureVerification default value to true to avoid breaking change#239

Closed
gmmcal wants to merge 1 commit into
SonarSource:masterfrom
gmmcal:default-skip-signature
Closed

SQSCANGHA-140 Set skipSignatureVerification default value to true to avoid breaking change#239
gmmcal wants to merge 1 commit into
SonarSource:masterfrom
gmmcal:default-skip-signature

Conversation

@gmmcal

@gmmcal gmmcal commented Apr 28, 2026

Copy link
Copy Markdown
Contributor

Address #235 (comment)

On #235, a new feature was introduce to check OpenPGP signature and improve security standards. Even though this is a great feature, it breaks the action execution when dirmngr is not present on runner. Shortly after feature was merged, remarks came into PR to point that out. Full error log is below

Installing Sonar Scanner CLI 8.0.1.6346 for linux-x64...
Downloading from: https://binaries.sonarsource.com/Distribution/sonar-scanner-cli/sonar-scanner-cli-8.0.1.6346-linux-x64.zip
Downloading signature from: https://binaries.sonarsource.com/Distribution/sonar-scanner-cli/sonar-scanner-cli-8.0.1.6346-linux-x64.zip.asc
Importing SonarSource public key from hkps://keyserver.ubuntu.com...
/usr/bin/gpg --homedir /home/runner/_work/_temp/gpg-home-1777397281878-365 --batch --keyserver hkps://keyserver.ubuntu.com --recv-keys 679F1EE92B19609DE816FDE81DB198F93525EC1A
gpg: keybox '/home/runner/_work/_temp/gpg-home-1777397281878-365/pubring.kbx' created
gpg: error running '/usr/bin/dirmngr': probably not installed
gpg: failed to start dirmngr '/usr/bin/dirmngr': Configuration error
gpg: can't connect to the dirmngr: Configuration error
gpg: keyserver receive failed: No dirmngr
Warning: Failed to import key from hkps://keyserver.ubuntu.com: The process '/usr/bin/gpg' failed with exit code 2
Attempting fallback keyserver hkps://keys.openpgp.org...
/usr/bin/gpg --homedir /home/runner/_work/_temp/gpg-home-1777397281878-365 --batch --keyserver hkps://keys.openpgp.org --recv-keys 679F1EE92B19609DE816FDE81DB198F93525EC1A
gpg: error running '/usr/bin/dirmngr': probably not installed
gpg: failed to start dirmngr '/usr/bin/dirmngr': Configuration error
gpg: can't connect to the dirmngr: Configuration error
gpg: keyserver receive failed: No dirmngr
Error: Action failed: Failed to import SonarSource public key from all keyservers. Primary (hkps://keyserver.ubuntu.com): The process '/usr/bin/gpg' failed with exit code 2. Fallback (hkps://keys.openpgp.org): The process '/usr/bin/gpg' failed with exit code 2

This PR aims to change default value of skipSignatureVerification to true, until a proper structural solution is in place within the action flow or Sonar team decides to mark this feature as a breaking change, turning default value to false on a v8 future release.

@sonar-review-alpha

sonar-review-alpha Bot commented Apr 28, 2026

Copy link
Copy Markdown

Summary

This PR flips the default value of skipSignatureVerification from false to true in action.yml. This restores backward compatibility after PR #235 introduced GPG signature verification that breaks on runners without dirmngr installed. The change is minimal—one boolean default and an updated description noting this is temporary. Users who want signature verification can still opt-in by setting skipSignatureVerification: "false" in their workflow.

What reviewers should know

What to review:

  • The default value change in action.yml (lines 28–29) — this is the core of the fix
  • The updated description clarifies the temporary nature and directs users how to enable verification

Context for reviewers:

  • This is a pragmatic short-term fix: signature verification is a valuable security feature, but it fails hard without dirmngr, breaking existing CI/CD pipelines
  • The expectation is that a proper solution (bundling dirmngr, removing the dependency, or marking as breaking for v8) will replace this default in the future
  • No code logic changes—this is purely configuration that will take effect immediately for existing workflows that don't override this input

Verification approach:

  • Confirm the description clearly communicates that this is temporary and how to opt back in
  • Consider whether the messaging is clear enough that users understand signature verification is still available if needed

  • Generate Walkthrough
  • Generate Diagram

🗣️ Give feedback

sonar-review-alpha[bot]

This comment was marked as resolved.

@gmmcal gmmcal force-pushed the default-skip-signature branch from d4650bf to a224a62 Compare April 28, 2026 19:39
@sonarqubecloud

Copy link
Copy Markdown

@sonar-review-alpha sonar-review-alpha Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The action.yml description fix is a clear improvement — the "not recommended for security" framing is gone and replaced with an accurate explanation of the temporary default. However, the PR missed a directly related piece: the runtime warning in src/main/install-sonar-scanner.js:61 still reads "⚠ Skipping GPG signature verification (not recommended)". With the default now true, this warning fires on every standard run for every user who hasn't explicitly set the input. Users will see a "not recommended" alarm for the behavior this PR explicitly designates as the safe, expected default — which directly undermines the goal of a smooth user experience. The warning text needs to be updated to match the intent (e.g., "⚠ GPG signature verification is disabled. Set skipSignatureVerification: false to enable it once dirmngr is available on your runner."). Update dist/index.js accordingly after changing the source.

🗣️ Give feedback

@antoine-vinot-sonarsource

antoine-vinot-sonarsource commented Apr 29, 2026

Copy link
Copy Markdown
Contributor

Hi @gmmcal,

Thank you for coming up with this fix 🚀

We will release a new patch version today (v7.2.1). Then we will switch the default back to false and release a major version (v8).

I cherry picked your commit in another branch, to validate with our CI.

I'm closing this PR in favor of #240

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