Skip to content

CLI-1520 Update MCP launch configuration - #1238

Merged
nquinquenel merged 3 commits into
masterfrom
bug/nq/maintenance-b
Nov 10, 2026
Merged

nquinquenel merged 3 commits into
masterfrom
bug/nq/maintenance-b

Conversation

@nquinquenel

@nquinquenel nquinquenel commented Nov 9, 2026 •

Copy link
Copy Markdown
Member

Updates how sonar run mcp prepares JVM configuration and manages temporary launch files.

  • Supplies selected proxy and TLS JVM settings through a temporary argument file mounted read-only into the container.
  • Supports the same configuration flow for native container runtimes and WSL.
  • Cleans up the temporary argument file on normal exit, startup failure, and SIGINT, SIGTERM, or SIGHUP, and removes the launch-specific signal handlers.
  • Adds coverage for JVM argument formatting, file lifecycle, and process termination, including a runtime that ignores the forwarded signal.

Validation: targeted unit and integration tests, type checking, linting, the binary build, and required analysis completed successfully. Real signal tests ran on Linux; native Windows was not validated.

@hashicorp-vault-sonar-prod hashicorp-vault-sonar-prod Bot changed the title Maintenance update B CLI-1520 Maintenance update B Nov 9, 2026
@hashicorp-vault-sonar-prod

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

Copy link
Copy Markdown
Contributor

CLI-1520

@nquinquenel nquinquenel changed the title CLI-1520 Maintenance update B Maintenance update B Nov 9, 2026
@netlify

netlify Bot commented Nov 9, 2026 •

Copy link
Copy Markdown

✅ Deploy Preview for sonarqube-cli canceled.

Name Link
🔨 Latest commit 4783e74
🔍 Latest deploy log https://app.netlify.com/projects/sonarqube-cli/deploys/6af2e132c1020f06a8c7e246

Comment thread src/commands/run/mcp.ts Resolved
@nquinquenel
nquinquenel marked this pull request as ready for review November 9, 2026 22:19
@nquinquenel
nquinquenel requested a review from a team as a code owner November 9, 2026 22:19
@nquinquenel nquinquenel changed the title Maintenance update B Update MCP launch configuration Nov 10, 2026
@nquinquenel nquinquenel changed the title Update MCP launch configuration CLI-1520 Update MCP launch configuration Nov 10, 2026
@nquinquenel
nquinquenel force-pushed the bug/nq/maintenance-b branch 2 times, most recently from f99c986 to 4783e74 Compare November 10, 2026 09:17
@gitar-bot

gitar-bot Bot commented Nov 10, 2026 •

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

🔴 High risk · Moving proxy and keystore credentials into temporary argument files changes sensitive secret handling; exposure or failed cleanup could leak credentials.

Review completed with no unresolved findings.

✅ 1 closed
✅ Security: Plaintext JVM secrets file outlives a signal-terminated CLI

📄 src/commands/run/mcp.ts:151-165 📄 src/commands/run/mcp.ts:194-195 📄 src/core/host/mcp/mcp-jvm-secrets.ts:54-68 📄 src/core/host/mcp/mcp-jvm-secrets.ts:99-113 📄 src/commands/CLAUDE.md:110
runMcp deletes the JVM secrets file only in its finally block (cleanup → removeJvmSecretsFile). There is no SIGTERM, SIGINT or SIGHUP handler anywhere in src/commands/run/mcp.ts, and the only process.on('SIGINT') in src is in import/server-backend.ts. So if an MCP client sends SIGTERM, or the user presses Ctrl-C on the process group, Bun exits immediately and finally never runs. The proxy username/password and PKCS#12 keystore passphrase then stay in plaintext at ~/.sonar/sonarqube-cli/cli-tmp/mcp-jvm-secrets-<pid>-<uuid>.args (mode 0644). Nothing removes the file until a later launch that writes another secrets file, and only once the file is over 15 minutes old.

Before this PR these credentials existed only in process env. The PR is what puts them on disk, and the new CLAUDE.md text says the file is removed after the child exits, which does not hold on these paths. Fix: register one-shot signal handlers around the spawn that run cleanup and then re-raise or exit with the signal's code. Alternatively, remove the file once the container has started, since the JVM reads the argfile only at launch.

Verification Rules 2/2 met Repos 2 inspected · PRs 1 inspected

Functional validation No results

Rules 2 passed 2 not applicable

✅ Internal artifact registries
✅ Repository license compliance

Broader impact Repos 2 inspected PRs 1 inspected
Cross-repo 2 repositories inspected 0 open impacts

SonarSource/sonarqube-agent-plugins Inspected
SonarSource/sonarqube-mcp-server Inspected

Cross-PR 1 PR inspected 0 open impacts

PR 1073: CLI-1330 Preserve proxy bypass for automatic MCP host aliases Inspected

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

@nquinquenel
nquinquenel merged commit b87988b into master Nov 10, 2026
27 checks passed
@nquinquenel
nquinquenel deleted the bug/nq/maintenance-b branch November 10, 2026 09:28
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