[PM-14993] Add ssh-agent error handling and security fixes

quexten commented 1 week ago

🎟️ Tracking

https://bitwarden.atlassian.net/browse/PM-14993 https://bitwarden.atlassian.net/browse/PM-14987 https://bitwarden.atlassian.net/browse/VULN-107

📔 Objective

This PR primarily aims to fix a security report. Socket creation should never happen in globally accessible directories (/tmp), so the fallback cannot be to /tmp. Instead, if the user has no home directory (which really should never be the case), we just error out. A user can manually overwrite this using the environment variable.

Further, this adds an error status to the ssh agent state. This allows us to detect if the ssh agent did not start up properly (f.e due to the socket not being createable, or the named pipe being used by the existing openssh service or 1password in windows). We can later (in a follow up PR) show the agent status in the settings UI, for now this just prevents crashes from trying to use the agent after creation failed.

📸 Screenshots

⏰ Reminders before review

Contributor guidelines followed
All formatters and local linters executed and passed
Written new unit and / or integration tests where applicable
Protected functional changes with optionality (feature flags)
Used internationalization (i18n) for all UI strings
CI builds passed
Communicated to DevOps any deployment requirements
Updated any necessary documentation (Confluence, contributing docs) or informed the documentation team

🦮 Reviewer guidelines

👍 (:+1:) or similar for great changes
📝 (:memo:) or ℹ️ (:information_source:) for notes or general info
❓ (:question:) for questions
🤔 (:thinking:) or 💭 (:thought_balloon:) for more open inquiry that's not quite a confirmed issue and could potentially benefit from discussion
🎨 (:art:) for suggestions / improvements
❌ (:x:) or ⚠️ (:warning:) for more significant problems or concerns needing attention
🌱 (:seedling:) or ♻️ (:recycle:) for future improvements or indications of technical debt
⛏ (:pick:) for minor or nitpick changes

github-actions[bot] commented 1 week ago

Checkmarx One – Scan Summary & Details – 9ea193f9-c892-484b-9c8f-d0015cd028b2

No New Or Fixed Issues Found

codecov[bot] commented 1 week ago

Codecov Report

Attention: Patch coverage is 0% with 2 lines in your changes missing coverage. Please review.

Project coverage is 33.45%. Comparing base (eda3885) to head (8f1190c). Report is 1 commits behind head on main.

:white_check_mark: All tests successful. No failed tests found.

Files with missing lines	Patch %	Lines
...esktop/src/platform/main/main-ssh-agent.service.ts	0.00%	2 Missing :warning:

Additional details and impacted files

```diff @@ Coverage Diff @@ ## main #12048 +/- ## ======================================= Coverage 33.45% 33.45% ======================================= Files 2863 2863 Lines 89638 89638 Branches 17059 17059 ======================================= Hits 29990 29990 Misses 57287 57287 Partials 2361 2361 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

🚨 Try these New Features:

JS Bundle Analysis - Avoid shipping oversized bundles

quexten commented 1 week ago

Merging since it's feature flagged.

bitwarden / clients