🛡️ Sentinel: [CRITICAL] Fix insecure secret storage in UserDefaults

[NSEvent]

Fixes an insecure secret storage issue identified by Sentinel by ensuring that legacy/backup secrets in UserDefaults are explicitly deleted regardless of KeychainService success.

---

PR created automatically by Jules for task 15582677634783491188 started by @NSEvent

Summary by CodeRabbit

• Bug Fixes
  • Improved cleanup of legacy credential storage during migration to ensure proper removal, even when migration encounters errors.

[google-labs-jules]

👋 Jules, reporting for duty! I'm here to lend a hand with this pull request.

When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down.

I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job!

For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with @jules. You can find this option in the Pull Request section of your global Jules UI settings. You can always switch back!

New to Jules? Learn more at jules.google/docs.

---

For security, I will only act on instructions from the user who triggered this task.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 8130def5-9d64-45de-a1b8-c9232af6edcf

📥 Commits

Reviewing files that changed from the base of the PR and between 2455a3c and 1176c7c.

📒 Files selected for processing (1)

• XboxControllerMapper/XboxControllerMapper/Services/Input/UniversalControlMouseRelay.swift

---

📝 Walkthrough

Walkthrough

This PR reorganizes the timing of insecure UserDefaults credential cleanup during Keychain migration. The relay shared secret deletion now occurs earlier and more reliably in both migration and storage flows, ensuring the plaintext copy is removed even when downstream operations fail.

Changes

Keychain Migration Security

Layer / File(s)	Summary
Keychain migration failure handling and deletion timing XboxControllerMapper/XboxControllerMapper/Services/Input/UniversalControlMouseRelay.swift	relaySharedSecretData() deletes the insecure UserDefaults copy before Keychain storage and treats nil from storePassword(...) as the failure condition to log; storeRelaySharedSecret(_:) moves the deletion of relaySharedSecretDefaultsKey to the function start, clearing the insecure copy before subsequent Keychain store and authenticator reset operations.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

• NSEvent/xbox-controller-mapper#47: Both PRs modify UniversalControlMouseRelay.swift to change how the universal relay shared secret's legacy plaintext UserDefaults copy is deleted during Keychain migration and storage in relaySharedSecretData() and storeRelaySharedSecret(_:).

Poem

🐰 A secret moves from shelf to vault so bright,
No trace of plaintext left to see the light!
Though Keychain stumble, cleanup stays the course—
The insecure whispers deleted by force. ✨

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately describes the main security fix: removing insecure secret storage from UserDefaults.
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.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch sentinel/fix-insecure-secret-storage-15582677634783491188

---

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 and usage tips.}