Skip to content

CLI-1504 Prevent repository-controlled scanner execution - #1234

Merged
vnaskos-sonar merged 6 commits into
masterfrom
bug/nq/CLI-1504-pin-scanner-execution
Nov 10, 2026
Merged

vnaskos-sonar merged 6 commits into
masterfrom
bug/nq/CLI-1504-pin-scanner-execution

Conversation

@nquinquenel

@nquinquenel nquinquenel commented Nov 9, 2026 •

Copy link
Copy Markdown
Member

sonar init could run repository-selected Java executables, JVM agents, engine JARs, or planted legacy cache JARs with SONAR_TOKEN. First analyses now pin the Java executable, JVM options, engine JAR, and scanner user home on the shared foreground/detached command line. The scanner home is private to each run, and all four properties are managed options.

Cloud and Server 10.6+ engines are downloaded using authenticated metadata and verified with SHA-256 before launch. Metadata download URLs receive no credentials. Older Servers retain legacy bootstrap with the isolated cache. Engine transfers have a ten-minute budget; metadata requests retain thirty seconds. Invalid versions or provisioning failures stop before launch. Ordinary root and module analysis properties still reach the scanner.

Validation:

  • 154 focused unit tests pass, including checksum rejection, regional/context routing, credential boundaries, failure before launch, authenticated binary redirects, and both download budgets.
  • Five compiled-CLI tests using SonarScanner 8.3.1 pass: modern escaped/continued properties in foreground, modern module properties detached, legacy root cache poisoning in foreground, legacy module cache poisoning detached, and an unsafe legacy cache control proving execution with the dummy token. Protected cases verify trusted completion and cleanup or detached retention.
  • Typecheck, edited-file ESLint, Prettier, and diff checks pass. Final DEEP Vortex analysis reports no issues.
  • Correctness, security, and ticket-fit reviews approved the implementation and test cleanup. The cleanup removes 110 net test lines and reduces the real-scanner matrix from thirteen cases to five.

CLI-1504


Summary by Gitar

  • Scanner discovery:
    • A scanner already on PATH is reused only when its runtime is Java 17 or newer; otherwise a managed install is used
    • Verification fails with a clear error when the launched scanner reports an unsupported Java version

This will update automatically on new commits.

@netlify

netlify Bot commented Nov 9, 2026 •

Copy link
Copy Markdown

✅ Deploy Preview for sonarqube-cli ready!

Name Link
🔨 Latest commit 5683166
🔍 Latest deploy log https://app.netlify.com/projects/sonarqube-cli/deploys/6af2e8bd5d12722544159895
😎 Deploy Preview https://deploy-preview-1234--sonarqube-cli.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.
🤖 Make changes Run an agent on this branch

To edit notification comments on pull requests, go to your Netlify project configuration.

@hashicorp-vault-sonar-prod

hashicorp-vault-sonar-prod Bot commented Nov 9, 2026 •

Copy link
Copy Markdown
Contributor

CLI-1504

Comment thread src/core/server/http-client.ts Resolved
@nquinquenel
nquinquenel marked this pull request as ready for review November 10, 2026 08:47
@nquinquenel
nquinquenel requested a review from a team as a code owner November 10, 2026 08:47
@tsiaraskon-sonar
tsiaraskon-sonar force-pushed the bug/nq/CLI-1504-pin-scanner-execution branch from e937221 to 5683166 Compare November 10, 2026 09:49
@gitar-bot

gitar-bot Bot commented Nov 10, 2026

Copy link
Copy Markdown
Code Review ✅ Approved 4 closed / 4 findings

🔴 High risk · Pins scanner Java, JVM options, engine JAR, and user home; a bypass could let repository-controlled code execute with SONAR_TOKEN.

Pins the Java executable, JVM options, engine JAR, and scanner user home on the scanner command line, and downloads Cloud and Server 10.6+ engines with authenticated metadata and SHA-256 verification before launch. Resolved the repository-controlled sonar.userHome cache planting, the 30s engine download cap now raised to a ten-minute budget, the redirect test that never reached the authenticated download, and first-scan fixtures lacking a Java version line. No issues found.

✅ 4 closed
✅ Security: sonar.userHome is still repository-controlled, so cached jars can be planted

📄 src/commands/init/scan/run-scanner.ts:105-113 📄 src/commands/init/scan/scanner-property.ts:45-55 📄 src/commands/init/scan/prepare-scanner-engine.ts:36-37 🔗 link 🔗 link
The PR pins javaExePath, javaOpts and engineJarPath, but it does not pin sonar.userHome. That key is also missing from MANAGED_PROPERTIES. In SonarScanner CLI 8.x, Conf.java merges sonar-project.properties into the bootstrap properties. ScannerEngineBootstrapper then reads sonar.userHome from those properties and puts the download cache at <sonarUserHome>/cache (ScannerEngineBootstrapper.java). On a cache hit, DownloadCache returns the existing <cache>/<hash>/<file> without hashing it again (DownloadCache.java).

The legacy bootstrap that this PR deliberately keeps for Server < 10.6 loads the /batch/index jars through that same cache into an in-process classloader (LegacyScannerEngineDownloader.java, IsolatedLauncherFactory.java).

Attack: a cloned repository sets sonar.userHome=.cache and commits .cache/cache/<hash>/<engine jar>. The hashes are public for each server version. That code then runs in the developer's process with SONAR_TOKEN, which is the exact outcome the PR says it prevents. Fix: pin -Dsonar.userHome= on the shared command line, for example to the trusted SONAR_USER_HOME or a private per-run directory, and add the key to MANAGED_PROPERTIES.

✅ Edge Case: Engine JAR download capped at 30s total, shorter than the scanner's

📄 src/core/server/http-client.ts:240-254 📄 src/commands/init/scan/scanner-engine-api.ts:26 📄 src/commands/init/scan/scanner-engine-api.ts:64-72
Both download paths abort the whole transfer, body included, after 30 seconds. getBytes uses GET_REQUEST_TIMEOUT_MS (30000) through AbortSignal.timeout, and downloadAnonymous uses ENGINE_DOWNLOAD_TIMEOUT_MS = 30_000. Before this PR the scanner downloaded the engine itself, with no total response timeout (sonar.scanner.responseTimeout=0) and only a 60s idle socket timeout (HttpConfig.java). On a slow link or proxy, a multi-megabyte engine JAR can now hit the deadline, and the first analysis fails before launch even though the scanner would have finished the download. Fix: give the engine download its own, much longer budget, or use an idle timeout instead of a total one.

✅ Quality: Redirect test never reaches the authenticated getBytes download

📄 tests/unit/commands/init/scanner-engine.test.ts:186-200 📄 src/core/server/http-client.ts:239-253
The test 'rejects a cross-origin redirect from the authenticated endpoint' makes every fetch return a 302. That means the first request, the JSON metadata http.get, already fails, and expect(fetchSpy).toHaveBeenCalledTimes(1) confirms nothing else runs. So the new getBytes path (Accept: application/octet-stream) is never sent a cross-origin redirect, even though its doc comment promises 'the same network and redirect protections'. Fix: return the metadata normally and send the 302 only for the octet-stream request, then assert that no request reached other.example.

✅ Bug: first-scan tests' fake java has no Java line, so they now hit managed install

📄 src/core/host/install/scanner-discovery.ts:85-90 📄 tests/unit/commands/init/first-scan.test.ts
verifyScanner() now rejects any --version output that has no Java <major> line (scanner-discovery.ts:85-90). installScanner() in tests/unit/commands/init/first-scan.test.ts replaces the fake launcher's java with a script that prints only SonarScanner CLI 8.3.1.16491 for --version. It does not use writeFakeJava, so the new javaVersion default never applies. In every first-scan test, probeLauncher() → verifyScanner() now throws. discoverSonarScanner() catches the error and returns null, and installSonarScanner() (steps/scan.ts:199, which is not mocked) falls back to downloading and installing the managed scanner. Those tests then never run the fake scanner, which they need to write the launched file, the ceTaskId report and the scanner says hello output. Depending on network access, they either fail or make real downloads. The fix is to add a Java 21.0.9 line to that fake's --version branch.

Verification Rules 2/2 met

Functional validation No results

Rules 2 passed 2 not applicable

✅ Internal artifact registries
✅ Repository license compliance

Auto-approval Withheld

For more verification details, comment gitar display:verbose.

Options

Auto-apply is off → Gitar will not commit updates to this branch.
Display: compact → Showing compact verification results.

Comment with these commands to change the behavior for this request:

Auto-apply Compact
gitar auto-apply:on         
gitar display:verbose         

Was this helpful? React with 👍 / 👎 | Gitar

@sonarqubecloud

Copy link
Copy Markdown

@vnaskos-sonar
vnaskos-sonar merged commit 561bfe8 into master Nov 10, 2026
27 checks passed
@vnaskos-sonar
vnaskos-sonar deleted the bug/nq/CLI-1504-pin-scanner-execution branch November 10, 2026 10:00
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.

3 participants