🛡️ Sentinel: [CRITICAL] Fix insecure plaintext storage fallback for OBS passwords

[NSEvent]

🚨 Severity: CRITICAL

💡 Vulnerability: The application was failing open by intentionally storing OBS WebSocket passwords in plaintext inside saved JSON structures (SystemCommand.swift and Macro.swift) if the secure Keychain storage operation failed.

🎯 Impact: If Keychain access failed due to permissions or environment issues, users' OBS passwords would be silently written to disk as plaintext inside exported profiles or internal configuration files, leading to sensitive credential disclosure.

🔧 Fix: Updated OBSWebSocketPayload encoding in both Macro.swift and SystemCommand.swift to fail securely. If KeychainService.storePassword fails, the application now logs an error and safely discards the password instead of falling back to encoding it in plaintext.

✅ Verification: Static analysis verified falling back to plaintext no longer exists anywhere in the codebase. Testing environment limitations prevented unit test execution natively, but the logic removal eliminates the insecure write path.

---

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

Summary by CodeRabbit

• Bug Fixes
  • Fixed a security vulnerability where OBS passwords could be stored in plaintext to disk when credential storage fails. The app now safely discards sensitive credentials instead of persisting them, enhancing protection against unauthorized access if configuration files are exported or accessed.

[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: 05182536-b620-4958-8fc5-512d13a0fad7

📥 Commits

Reviewing files that changed from the base of the PR and between 78ad10c and a32705b.

📒 Files selected for processing (3)

• .Jules/sentinel.md
• XboxControllerMapper/XboxControllerMapper/Models/Macros/Macro.swift
• XboxControllerMapper/XboxControllerMapper/Models/SystemCommand.swift

---

📝 Walkthrough

Walkthrough

In Macro.swift and SystemCommand.swift, the plaintext password fallback triggered when KeychainService.storePassword returns nil is removed; the credential is now discarded instead of being written to the JSON payload. The .Jules/sentinel.md security log records this vulnerability and its resolution.

Changes

OBS Password Plaintext Fallback Removal

Layer / File(s)	Summary
Secure Keychain failure path and sentinel log XboxControllerMapper/.../Models/Macros/Macro.swift, XboxControllerMapper/.../Models/SystemCommand.swift, .Jules/sentinel.md	Both OBSWebSocketPayload and .obsWebSocket encode(to:) paths no longer fall back to encoding the plaintext password when KeychainService.storePassword returns nil; the credential is discarded and a security log message is emitted. The sentinel log documents the insecure plaintext fallback storage issue and the secure-fail directive.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~5 minutes

Possibly related PRs

• NSEvent/xbox-controller-mapper#47: Also updates Keychain persistence failure handling to prevent sensitive credentials from being written to disk in plaintext, targeting the Universal Control Relay shared secret in UserDefaults.
• NSEvent/xbox-controller-mapper#54: Changes KeychainService.storePassword failure handling to stop persisting secrets in plaintext, specifically removing a legacy plaintext secret from UserDefaults during migration.

Poem

🐰 No plaintext for me, no siree!
When Keychain says no, I set the secret free —
Discarded, not written, not saved to a file,
A hop toward security done in great style.
The sentinel log marks the day I stood tall:
Fail safe, little password — or don't store at all! 🔐

🚥 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 clearly identifies the main change: fixing insecure plaintext storage fallback for OBS passwords, which is the primary focus of all file modifications in this changeset.
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-plaintext-fallback-4635798464128746884

---

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.}