Skip to content

SQSCANGHA-141 fix: resolve 9 SonarQube issues in gpg-verification files from #235#236

Closed
sonarqube-agent[bot] wants to merge 2 commits into
claire/SQSCANGHA-140/signaturefrom
remediate-claire/SQSCANGHA-140/signature-20260427-161050-883af3a8
Closed

SQSCANGHA-141 fix: resolve 9 SonarQube issues in gpg-verification files from #235#236
sonarqube-agent[bot] wants to merge 2 commits into
claire/SQSCANGHA-140/signaturefrom
remediate-claire/SQSCANGHA-140/signature-20260427-161050-883af3a8

Conversation

@sonarqube-agent

Copy link
Copy Markdown

Addresses quality gate issues from #235.

Updated Node.js built-in module imports to use the node: protocol prefix (fs, os, path) in both gpg-verification.js and its test file, replaced global parseInt with Number.parseInt, and added error logging to an empty catch block. These changes align with modern Node.js best practices, improve code clarity and security, and ensure exceptions are properly handled rather than silently ignored.

View Project in SonarCloud


Fixed Issues

javascript:S7772 - Prefer `node:fs` over `fs`. • MINORView issue

Location: src/main/gpg-verification.js:23

What changed

This hunk adds the node: protocol prefix to three Node.js built-in module imports in src/main/gpg-verification.js. It changes "fs" to "node:fs", "os" to "node:os", and "path" to "node:path". This directly resolves the static analysis warnings about preferring the node: protocol for built-in module imports, which improves clarity (making it obvious these are core Node.js modules rather than third-party packages), enhances security (preventing potential confusion with similarly-named npm packages), and aligns with modern Node.js best practices.

--- a/src/main/gpg-verification.js
+++ b/src/main/gpg-verification.js
@@ -23,3 +23,3 @@ import * as exec from "@actions/exec";
-import * as fs from "fs";
-import * as os from "os";
-import * as path from "path";
+import * as fs from "node:fs";
+import * as os from "node:os";
+import * as path from "node:path";
javascript:S7772 - Prefer `node:os` over `os`. • MINORView issue

Location: src/main/gpg-verification.js:24

What changed

This hunk adds the node: protocol prefix to three Node.js built-in module imports in src/main/gpg-verification.js. It changes "fs" to "node:fs", "os" to "node:os", and "path" to "node:path". This directly resolves the static analysis warnings about preferring the node: protocol for built-in module imports, which improves clarity (making it obvious these are core Node.js modules rather than third-party packages), enhances security (preventing potential confusion with similarly-named npm packages), and aligns with modern Node.js best practices.

--- a/src/main/gpg-verification.js
+++ b/src/main/gpg-verification.js
@@ -23,3 +23,3 @@ import * as exec from "@actions/exec";
-import * as fs from "fs";
-import * as os from "os";
-import * as path from "path";
+import * as fs from "node:fs";
+import * as os from "node:os";
+import * as path from "node:path";
javascript:S7772 - Prefer `node:path` over `path`. • MINORView issue

Location: src/main/gpg-verification.js:25

What changed

This hunk adds the node: protocol prefix to three Node.js built-in module imports in src/main/gpg-verification.js. It changes "fs" to "node:fs", "os" to "node:os", and "path" to "node:path". This directly resolves the static analysis warnings about preferring the node: protocol for built-in module imports, which improves clarity (making it obvious these are core Node.js modules rather than third-party packages), enhances security (preventing potential confusion with similarly-named npm packages), and aligns with modern Node.js best practices.

--- a/src/main/gpg-verification.js
+++ b/src/main/gpg-verification.js
@@ -23,3 +23,3 @@ import * as exec from "@actions/exec";
-import * as fs from "fs";
-import * as os from "os";
-import * as path from "path";
+import * as fs from "node:fs";
+import * as os from "node:os";
+import * as path from "node:path";
javascript:S7772 - Prefer `node:fs` over `fs`. • MINORView issue

Location: src/main/__tests__/gpg-verification.test.js:23

What changed

This hunk adds the node: protocol prefix to three Node.js built-in module imports: fs becomes node:fs, path becomes node:path, and os becomes node:os. This makes it explicitly clear that these are core Node.js modules rather than potentially third-party npm packages, improving clarity, security, and alignment with modern Node.js best practices.

--- a/src/main/__tests__/gpg-verification.test.js
+++ b/src/main/__tests__/gpg-verification.test.js
@@ -23,3 +23,3 @@ import assert from "node:assert/strict";
-import * as fs from "fs";
-import * as path from "path";
-import * as os from "os";
+import * as fs from "node:fs";
+import * as path from "node:path";
+import * as os from "node:os";
javascript:S7772 - Prefer `node:path` over `path`. • MINORView issue

Location: src/main/__tests__/gpg-verification.test.js:24

What changed

This hunk adds the node: protocol prefix to three Node.js built-in module imports: fs becomes node:fs, path becomes node:path, and os becomes node:os. This makes it explicitly clear that these are core Node.js modules rather than potentially third-party npm packages, improving clarity, security, and alignment with modern Node.js best practices.

--- a/src/main/__tests__/gpg-verification.test.js
+++ b/src/main/__tests__/gpg-verification.test.js
@@ -23,3 +23,3 @@ import assert from "node:assert/strict";
-import * as fs from "fs";
-import * as path from "path";
-import * as os from "os";
+import * as fs from "node:fs";
+import * as path from "node:path";
+import * as os from "node:os";
javascript:S7772 - Prefer `node:os` over `os`. • MINORView issue

Location: src/main/__tests__/gpg-verification.test.js:25

What changed

This hunk adds the node: protocol prefix to three Node.js built-in module imports: fs becomes node:fs, path becomes node:path, and os becomes node:os. This makes it explicitly clear that these are core Node.js modules rather than potentially third-party npm packages, improving clarity, security, and alignment with modern Node.js best practices.

--- a/src/main/__tests__/gpg-verification.test.js
+++ b/src/main/__tests__/gpg-verification.test.js
@@ -23,3 +23,3 @@ import assert from "node:assert/strict";
-import * as fs from "fs";
-import * as path from "path";
-import * as os from "os";
+import * as fs from "node:fs";
+import * as path from "node:path";
+import * as os from "node:os";
javascript:S2486 - Handle this exception or don't catch it at all. • MINORView issue

Location: src/main/__tests__/gpg-verification.test.js:42

What changed

This hunk fixes the empty catch block that silently swallowed exceptions during test cleanup. Instead of just having a comment saying 'Ignore cleanup errors', it now logs the error via console.error, ensuring the exception is properly handled rather than completely ignored. This satisfies the rule that caught exceptions should be handled or logged rather than silently discarded.

--- a/src/main/__tests__/gpg-verification.test.js
+++ b/src/main/__tests__/gpg-verification.test.js
@@ -43,1 +43,2 @@ describe("gpg-verification", () => {
-        // Ignore cleanup errors
+        // Intentionally ignoring cleanup errors in test teardown
+        console.error(`Failed to clean up temp directory: ${error.message}`);
javascript:S7773 - Prefer `Number.parseInt` over `parseInt`. • MINORView issue

Location: src/main/__tests__/gpg-verification.test.js:67

What changed

This hunk replaces both usages of the global parseInt function with Number.parseInt. The first occurrence on the line computing mode from stats.mode & parseInt('777', 8) is changed to Number.parseInt('777', 8), fixing the first warning about preferring Number.parseInt over the global parseInt. The second occurrence on the assertion line assert.equal(mode, parseInt('700', 8)) is changed to Number.parseInt('700', 8), fixing the second warning about preferring Number.parseInt over the global parseInt. Both changes align with modern ES2015+ best practices by using the Number namespace equivalent instead of the global function.

--- a/src/main/__tests__/gpg-verification.test.js
+++ b/src/main/__tests__/gpg-verification.test.js
@@ -67,2 +68,2 @@ describe("gpg-verification", () => {
-        const mode = stats.mode & parseInt("777", 8);
-        assert.equal(mode, parseInt("700", 8));
+        const mode = stats.mode & Number.parseInt("777", 8);
+        assert.equal(mode, Number.parseInt("700", 8));
javascript:S7773 - Prefer `Number.parseInt` over `parseInt`. • MINORView issue

Location: src/main/__tests__/gpg-verification.test.js:68

What changed

This hunk replaces both usages of the global parseInt function with Number.parseInt. The first occurrence on the line computing mode from stats.mode & parseInt('777', 8) is changed to Number.parseInt('777', 8), fixing the first warning about preferring Number.parseInt over the global parseInt. The second occurrence on the assertion line assert.equal(mode, parseInt('700', 8)) is changed to Number.parseInt('700', 8), fixing the second warning about preferring Number.parseInt over the global parseInt. Both changes align with modern ES2015+ best practices by using the Number namespace equivalent instead of the global function.

--- a/src/main/__tests__/gpg-verification.test.js
+++ b/src/main/__tests__/gpg-verification.test.js
@@ -67,2 +68,2 @@ describe("gpg-verification", () => {
-        const mode = stats.mode & parseInt("777", 8);
-        assert.equal(mode, parseInt("700", 8));
+        const mode = stats.mode & Number.parseInt("777", 8);
+        assert.equal(mode, Number.parseInt("700", 8));

Have a suggestion or found an issue? Share your feedback here.


SonarQube Remediation Agent uses AI. Check for mistakes.

DISCLAIMER: Remediation Agent will not be triggered again on this (self authored) PR

…ipt:S2486

Commit 1 of SonarQube suggestions

Fully fixed issues:
- [javascript:S7773] AZ3Pp_T3hKtKf9aAx2tr: Prefer `Number.parseInt` over `parseInt`.
- [javascript:S7773] AZ3Pp_T3hKtKf9aAx2ts: Prefer `Number.parseInt` over `parseInt`.
- [javascript:S7772] AZ3Pp_T3hKtKf9aAx2tn: Prefer `node:fs` over `fs`.
- [javascript:S7772] AZ3Pp_T3hKtKf9aAx2to: Prefer `node:path` over `path`.
- [javascript:S7772] AZ3Pp_T3hKtKf9aAx2tp: Prefer `node:os` over `os`.
- [javascript:S2486] AZ3Pp_T3hKtKf9aAx2tq: Handle this exception or don't catch it at all.

Generated by SonarQube Agent
Commit 2 of SonarQube suggestions

Fully fixed issues:
- [javascript:S7772] AZ3Pp_V9hKtKf9aAx2tt: Prefer `node:fs` over `fs`.
- [javascript:S7772] AZ3Pp_V9hKtKf9aAx2tu: Prefer `node:os` over `os`.
- [javascript:S7772] AZ3Pp_V9hKtKf9aAx2tv: Prefer `node:path` over `path`.

Generated by SonarQube Agent
@sonarqubecloud

Copy link
Copy Markdown

@hashicorp-vault-sonar-prod hashicorp-vault-sonar-prod Bot changed the title fix: resolve 9 SonarQube issues in gpg-verification files from #235 SQSCANGHA-141 fix: resolve 9 SonarQube issues in gpg-verification files from #235 Apr 27, 2026
@hashicorp-vault-sonar-prod

hashicorp-vault-sonar-prod Bot commented Apr 27, 2026

Copy link
Copy Markdown

SQSCANGHA-141

@sonar-review-alpha

sonar-review-alpha Bot commented Apr 27, 2026

Copy link
Copy Markdown

Summary

⚠️ The PR description exceeded the analysis limit and was truncated. The review may not reflect all context.

This PR resolves 9 SonarQube code quality issues in the GPG verification module by modernizing Node.js best practices:

  • Updated built-in module imports across gpg-verification.js and its test file to use the explicit node: protocol prefix (node:fs, node:os, node:path). This clarifies intent, prevents potential naming collisions with npm packages, and aligns with modern Node.js conventions.
  • Replaced global parseInt with Number.parseInt in the test file for better code clarity and to avoid potential scope confusion.
  • Added error logging to an empty catch block in test teardown so cleanup failures are visible rather than silently ignored.

No functional changes — all modifications are code quality improvements.

What reviewers should know

Review scope: Changes are isolated to two files (main module + tests) and are purely about code style/quality.

What to check:

  • Verify all 6 import statements now use the node: prefix consistently
  • Confirm Number.parseInt calls include the radix argument (they do: base 8 for octal permission checks)
  • The error logging in the test catch block is minimal and appropriate for test cleanup — not a functional change

Low-risk PR: No logic changes, no new dependencies, no breaking changes. Safe to approve once the changes are verified.


  • Generate Walkthrough
  • Generate Diagram

🗣️ Give feedback

@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.

LGTM! ✅

Clean PR — the changes are correct and safe.

One thing worth knowing: 8 other source files under src/ still use bare specifiers without the node: prefix — run-sonar-scanner.js, sanity-checks.js, install-sonar-scanner.js, and the install-build-wrapper module. The scope here is intentionally limited to the files flagged in #235, but those remaining files will likely surface the same S7772 rule on the next scan and need a follow-up.

🗣️ Give feedback

@sonarqube-agent sonarqube-agent AI closed this Apr 27, 2026
@sonarqube-agent sonarqube-agent AI deleted the remediate-claire/SQSCANGHA-140/signature-20260427-161050-883af3a8 branch April 27, 2026 16:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant