Security: harden local key file writes, use SFTP for remote file access

[frequency403]

Summary

Fixes two security findings in how the app reads and writes files.

Local (KeyFileWriterService, KeyFileBackupService, SshConfigFilesExtension)

• When overwriting, the file was opened with FileMode.OpenOrCreate, which does not truncate it. If the new key was shorter (for example, the passphrase was removed), bytes of the old key stayed at the end of the file and the key was corrupt. The writer now uses FileMode.Create to overwrite and FileMode.CreateNew otherwise; CreateNew also closes the race between the existence check and the open.
• UnixCreateMode only applies when a file is created. An existing file with 0644 kept those permissions even after a private key was written into it. The writer now sets 0600 explicitly on the file handle.
• FileShare.ReadWrite is now FileShare.None.
• The backup directory ~/.ssh/<App>, which holds copies of the private keys, and a newly created ~/.ssh are now created with 0700.

Remote (ServerConnection)

• Previously, shell commands were built from server-provided paths (the result of echo $HOME, unquoted in cat/touch) and from file contents. On Windows, the content ended up inside powershell -Command "...", so a " in authorized_keys (for example, command="..." options) broke the quoting. That corrupts the file or lets an entry inject commands.
• authorized_keys and known_hosts are now read and written via SftpClient, using paths relative to the SFTP home directory. No file path or file content goes through a shell anymore, and the command-line length limit no longer applies.
• Files are no longer created eagerly on connect. A missing file reads as empty. When it is written for the first time, it is created with 0600 (and the directory with 0700) on Unix hosts.
• Side effects: Set-Content -Encoding UTF8 in PowerShell 5.1 wrote a BOM; the file is now written as UTF-8 without BOM. ServerConnection.Dispose now also disposes the SSH and SFTP clients.

Changes Made

• Bug fix (non-breaking change)
• New feature (non-breaking change)
• Breaking change (fix or feature that would cause existing functionality to change)
• Documentation update
• Refactoring (no functional change)

Testing Performed

• Tested on Windows (Version: _______)
• Tested on macOS (Version: _______)
• Tested on Linux (unit tests only, .NET SDK 10.0.112)
• Added unit tests (if applicable)
• Verified no regression in existing functionality

Test Evidence

• New file KeyFileWriterServiceTests with 4 tests: truncation on overwrite, no overwrite without the flag, 0600 on an existing 0644 file, 0600 on a new file.
• Ran against the old implementation, the truncation and permissions tests fail (2 failed). With the fix, all 207 tests pass.
• The SFTP path has no automated test (it needs an SSH server). It should be checked manually against a Linux server and a Windows OpenSSH server.

Checklist

• Code follows project style guidelines
• Self-reviewed my code
• Commented difficult areas
• Updated documentation (if needed)
• No new warnings/errors introduced
• All builds pass locally (dotnet build, dotnet test)

Questions for Reviewers

• This requires the SFTP subsystem on the target server. It is enabled by default in OpenSSH on Linux and Windows. Is a shell fallback wanted anyway?

---

Notes for Reviewers:

• Priority: high
• Breaking change: no (requires the SFTP subsystem on the server)

🤖 Generated with Claude Code

https://claude.ai/code/session_01XSUJwZ17AWMdDvYXboJAbQ

---

Generated by Claude Code