fix: fall back to insecure storage when Linux Secret Service is not activatable

[SathyaBhat]

Summary

• isAvailable() in the Linux credential adapter returned true when secret-tool lookup exited with code 1 and a D-Bus error in stderr (e.g. The name is not activatable), because the probe only checked exit code. A healthy "not found" response also exits 1 but produces no stderr, so the fix checks that stderr is empty before reporting available.
• Added a try/catch around store.set() in persistSiteCredential so that if set() throws despite isAvailable() returning true (race or stale probe), the error still falls through to plaintext storage when --insecure-storage was passed, rather than surfacing an unhandled store error.

Test plan

• linux adapter returns unavailable when probe exits code 1 with D-Bus stderr — unit test in lib-credentials.test.ts
• linux adapter considers service available only when probe exits cleanly with no stderr — confirms the healthy "not found" path still works
• falls back to plaintext when store reports available but set throws and --insecure-storage is passed — integration test in credentials-and-security.test.ts
• re-throws store set error without --insecure-storage when store reports available but set throws — integration test confirming error surfaces correctly without the flag
• All four new tests pass; pre-existing test suite unaffected by these changes

🤖 Generated with Claude Code

[coderabbitai]

Walkthrough

Credential persistence now falls back to plaintext storage when secure credential writes fail and insecure storage is enabled, while preserving error propagation otherwise. Linux Secret Service availability now requires an accepted exit code and empty stderr. Tests and a broken credential-store helper cover write failures, fallback behavior, error handling, and Linux probe outcomes.

Estimated code review effort: 3 (Moderate) | ~20 minutes

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title matches the main change: falling back to insecure storage when Linux Secret Service is unavailable.
Description check	✅ Passed	The description accurately summarizes the Linux availability probe fix, fallback behavior, and added tests.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

🧹 Nitpick comments (1)

src/commands/auth.ts (1)

360-370: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Consider logging a warning when falling back to plaintext storage.

When store.set() fails and --insecure-storage is enabled, the error is silently swallowed and the user gets no indication that secure storage was attempted and failed. A brief warning would help users diagnose intermittent Secret Service issues.

♻️ Optional: warn on fallback

     } catch (err) {
       if (!allowInsecureStorage) {
         throw err;
       }
-      // store.set() failed even though isAvailable() returned true (e.g. D-Bus
-      // service became unavailable between probe and write). Fall through to
-      // plaintext storage since --insecure-storage was explicitly requested.
+      // store.set() failed even though isAvailable() returned true (e.g. D-Bus
+      // service became unavailable between probe and write). Fall through to
+      // plaintext storage since --insecure-storage was explicitly requested.
+      console.warn(`Warning: secure credential storage failed (${err instanceof Error ? err.message : String(err)}). Falling back to plaintext storage.`);
     }

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/commands/auth.ts` around lines 360 - 370, In the catch block surrounding
store.set in the authentication flow, log a brief warning before falling through
to plaintext storage when allowInsecureStorage is enabled. Include that secure
storage failed and plaintext fallback is being used, while preserving the
existing rethrow behavior when insecure storage is not allowed.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@src/commands/auth.ts`:
- Around line 360-370: In the catch block surrounding store.set in the
authentication flow, log a brief warning before falling through to plaintext
storage when allowInsecureStorage is enabled. Include that secure storage failed
and plaintext fallback is being used, while preserving the existing rethrow
behavior when insecure storage is not allowed.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: ed4ac7c5-3737-4e36-b828-f37a5131e3be

📥 Commits

Reviewing files that changed from the base of the PR and between fb05259 and 893a0e0.

📒 Files selected for processing (5)

• src/commands/auth.ts
• src/lib/credentials.ts
• tests/credentials-and-security.test.ts
• tests/helpers/mock-credentials.ts
• tests/lib-credentials.test.ts