[nquinquenel]

---

Summary by Gitar

• Authentication hardening:
  • Validate browser authentication tokens via checkTokenStatus during callback handling and token generation
  • Reject unauthorized browser tokens with a 401 status and support fallback to manual token input
  • Update recordConnectionFromAuth to correctly set the envOnly marker on existing matching connections

_{This will update automatically on new commits.}

[netlify]

✅ Deploy Preview for sonarqube-cli canceled.

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

[hashicorp-vault-sonar-prod]

CLI-1082

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues
0 New dependency risks

Measures
0 Security Hotspots
86.5% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

[gitar-bot]

Code Review ✅ Approved 6 closed / 6 findings

🔴 High risk · Authentication flow changes alter browser token acceptance and login behavior.

Hardens authentication flows by validating browser authentication tokens via checkTokenStatus during callback handling and token generation, rejecting unauthorized tokens with a 401 status and supporting fallback to manual input. Resolves six issues: envOnly marker stamping, callback token rejection, server reachability validation, keychain error handling, CI login test responsiveness, and envOnly documentation.

✅ 6 closed

✅ Bug: envOnly marker stamped on login connection disables logout

📄 src/core/auth/auth-connection-recorder.ts:65-68
After sonar auth login the active connection is fully enriched (userUuid + sqsInstallationId), so any later command run with SONARQUBE_CLI_TOKEN/SONARQUBE_CLI_SERVER pointing at the same server now takes the early-return branch and permanently persists envOnly: true on that keychain-backed connection (before this PR the branch returned without mutating). A subsequent sonar auth logout (env vars unset) hits active.envOnly in logout.ts:43, prints "You are already logged out." and never revokes the server token, never calls deleteToken, and leaves isAuthenticated: true — so the keychain token stays usable and the user believes they are logged out. Only mark envOnly when there is no keychain token behind the connection.

✅ Edge Case: Rejected callback token leaves CI login hanging forever

📄 src/core/auth/token.ts:268-275 📄 src/core/auth/token.ts:286-299
The callback now only calls resolveToken when validation returns valid, so a browser-delivered token that is invalid — or valid but whose validation call returns unreachable because of a transient network/proxy error — leaves tokenPromise unresolved. In the CI === 'true' branch (token.ts:290) there is no timeout, abort signal or race, and no interactive paste fallback, so the CLI blocks indefinitely with no diagnostic instead of failing; previously the POST always resolved the promise. Fail fast in the non-interactive branch (and log the rejection) rather than waiting forever.

✅ Edge Case: Unreachable server reported as an unvalidatable token

📄 src/core/auth/token.ts:269 📄 src/core/auth/token.ts:301-308
checkTokenStatus returns unreachable for any network/HTTP failure (token.ts:66-73), and both new call sites treat anything other than valid as a bad token: the callback returns 401 and the final check throws The provided token could not be validated by the SonarQube server. So a manually pasted, perfectly good token fails login on a transient outage, with a message blaming the token and no remediationHint, while preflight-summary.ts distinguishes the two statuses. Branch on unreachable and say the server could not be reached.

✅ Bug: Env-var auth now dies silently when the keychain is unreachable

📄 src/core/auth/auth-connection-recorder.ts:60-64
recordConnectionFromAuth now calls getToken() on the env-var path, and getToken propagates a CommandFailedError whenever the backend fails (wrapBunSecrets, keychain.ts:47) — which is exactly what happens in headless containers/CI where Bun.secrets has no libsecret/D-Bus and no SONARQUBE_CLI_KEYCHAIN_FILE is set. The only caller passing envOnly: true (recordEnvAuthConnectionOnce, auth-resolver.ts:90) swallows that rejection into a logger.debug, so with SONARQUBE_CLI_TOKEN + SONARQUBE_CLI_SERVER on such a machine the connection is no longer recorded at all: state.auth.isAuthenticated stays false, no connection row, no telemetry identity — the very behaviour tests/integration/specs/auth/env-auth-state-sync.test.ts was written to guarantee (integration tests always set the file backend, so they never exercise this). Keychain-free env-var auth is the documented fallback for a broken keychain (KEYCHAIN_UNAVAILABLE_HINT), so the lookup must not be able to break it: catch the error and treat it as "cannot prove this is env-only".

✅ Bug: New CI login test never answers the trust prompt, so it can't pass

📄 tests/integration/specs/auth/auth.test.ts:135-148
auth login --server <fake on-premise url> hits confirmServerTrust → confirmPrompt('Connect to: …?') before it ever reaches the browser flow, and confirmPrompt has no non-TTY/CI shortcut (unlike pressEnterKeyPrompt). harness.run() pipes stdin and never writes to it, so the prompt either cancels (Login cancelled on stderr → the toContain('The token delivered by the browser could not be validated.') assertion fails) or blocks until the harness timeout; the invalid browserToken is never delivered. Every other on-premise login test in this file goes through the confirmTrust helper for this reason — use it here too.

...and 1 more closed from earlier reviews

Verification

Functional validation No results

Rules No rules evaluated

Auto-approval Not enabled

Set up

Options

Auto-apply is off → Gitar will not commit updates to this branch.
Display: compact → Counting what did not apply, without listing it.

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}