fix(onboarding): make chmod hardening best-effort (#897)
When the onboarding flow writes the API key under `~/.deepseek/`, the parent-dir and config-file chmod are propagated as hard errors if they fail. On Docker-on-Windows where the container mounts `$USERPROFILE\ .deepseek` to `/home/deepseek/.deepseek`, the bind-mounted NTFS volume can't accept Unix chmod, so `set_permissions` returns EPERM and the user sees `Failed to save API key: Failed to set permissions on /home/deepseek/.deepseek` — even though the directory and the secret file were already created successfully. The chmod is a hardening pass: the dir already lives under the user's home and the file is created with `O_CREAT | mode(0o600)` via OpenOptions. On filesystems where Unix permissions don't apply at all (Docker bind-mount of NTFS, network shares, FAT, certain CI volumes), the host's native ACL model is doing access control regardless. So demote the chmod to best-effort on Unix: warn loudly via `tracing::warn!` for security-sensitive operators who run with `RUST_LOG=warn`, then continue. Three sites: - `config::ensure_parent_dir` — parent-dir 0o700 hardening - `config::write_config_file_secure` — file 0o600 hardening - `secrets::FileKeyringStore::store_unlocked` — file 0o600 hardening (the parent-dir chmod here was already best-effort via `let _ =`) Tests: - `cargo test -p deepseek-secrets --all-features --locked` → 16/16 pass (including `file_store_round_trips_with_secure_perms` which still asserts mode 0o600 on a normal Linux test FS) - `cargo test -p deepseek-tui --bin deepseek-tui --all-features save_api_key --locked` → 5/5 pass - `cargo clippy -p deepseek-tui -p deepseek-secrets --all-features --locked -- -D warnings` clean - `cargo fmt --all -- --check` clean The GitBash paste failure mode reported in the same issue is a terminal quirk (GitBash on Windows doesn't reliably forward Ctrl+V to TUI apps); PowerShell + Shift+Insert work, as the reporter discovered. Not in scope here. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
This commit is contained in:
@@ -266,9 +266,17 @@ impl FileKeyringStore {
|
||||
#[cfg(unix)]
|
||||
{
|
||||
use std::os::unix::fs::PermissionsExt;
|
||||
let mut perms = fs::metadata(&self.path)?.permissions();
|
||||
perms.set_mode(0o600);
|
||||
fs::set_permissions(&self.path, perms)?;
|
||||
// Best-effort 0o600 — matches the parent-dir chmod above which
|
||||
// is also `let _ = ...`. Filesystems that don't support Unix
|
||||
// chmod (Docker bind-mounts of NTFS, network shares — #897)
|
||||
// would otherwise fail the whole save here even though the
|
||||
// blob already wrote successfully. The host's native ACLs
|
||||
// are doing access control in those environments.
|
||||
if let Ok(meta) = fs::metadata(&self.path) {
|
||||
let mut perms = meta.permissions();
|
||||
perms.set_mode(0o600);
|
||||
let _ = fs::set_permissions(&self.path, perms);
|
||||
}
|
||||
}
|
||||
Ok(())
|
||||
}
|
||||
|
||||
@@ -2289,14 +2289,32 @@ pub fn ensure_parent_dir(path: &Path) -> Result<()> {
|
||||
.with_context(|| format!("Failed to create directory: {}", parent.display()))?;
|
||||
#[cfg(unix)]
|
||||
{
|
||||
let mut perms = fs::metadata(parent)
|
||||
.with_context(|| format!("Failed to read metadata for {}", parent.display()))?
|
||||
.permissions();
|
||||
if perms.mode() & 0o077 != 0 {
|
||||
perms.set_mode(perms.mode() & !0o077);
|
||||
fs::set_permissions(parent, perms).with_context(|| {
|
||||
format!("Failed to set permissions on {}", parent.display())
|
||||
})?;
|
||||
// Tighten group/other bits on the parent dir as a hardening pass.
|
||||
// The dir lives under the user's home, so the chmod is best-effort:
|
||||
// filesystems that don't accept Unix permission bits (Docker
|
||||
// bind-mounts of NTFS, network shares, FAT, certain CI volumes —
|
||||
// see #897) return EPERM/ENOTSUP. The dir already exists by the
|
||||
// time we get here, so failing the whole save just because we
|
||||
// couldn't tighten perms strands the user mid-onboarding. Warn
|
||||
// loudly so a security-sensitive operator can still notice via
|
||||
// `RUST_LOG=warn`, then continue.
|
||||
if let Ok(meta) = fs::metadata(parent) {
|
||||
let mode = meta.permissions().mode();
|
||||
if mode & 0o077 != 0 {
|
||||
let mut perms = meta.permissions();
|
||||
perms.set_mode(mode & !0o077);
|
||||
if let Err(err) = fs::set_permissions(parent, perms) {
|
||||
tracing::warn!(
|
||||
target: "deepseek::config",
|
||||
path = %parent.display(),
|
||||
error = %err,
|
||||
"could not tighten parent dir permissions; \
|
||||
filesystem may not support Unix chmod \
|
||||
(Docker bind-mount, NTFS, network share). \
|
||||
Continuing — the file will still be written."
|
||||
);
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -2315,7 +2333,24 @@ fn write_config_file_secure(path: &Path, content: &str) -> Result<()> {
|
||||
.mode(0o600)
|
||||
.open(path)?;
|
||||
file.write_all(content.as_bytes())?;
|
||||
file.set_permissions(fs::Permissions::from_mode(0o600))?;
|
||||
// The file was already opened with mode 0o600; the explicit
|
||||
// set_permissions re-asserts that on filesystems where mode-at-open
|
||||
// didn't take effect (or where the file already existed with broader
|
||||
// bits). Filesystems that don't accept Unix chmod at all (Docker
|
||||
// bind-mounts of NTFS, network shares — #897) return EPERM. Treat
|
||||
// that as a warning rather than failing the whole save: the file
|
||||
// contents are written, and on Windows/macOS hosts the parent file
|
||||
// system's native ACL model is doing the access control.
|
||||
if let Err(err) = file.set_permissions(fs::Permissions::from_mode(0o600)) {
|
||||
tracing::warn!(
|
||||
target: "deepseek::config",
|
||||
path = %path.display(),
|
||||
error = %err,
|
||||
"could not enforce 0o600 on config file; filesystem may \
|
||||
not support Unix chmod. File contents written; rely on \
|
||||
host ACLs for access control."
|
||||
);
|
||||
}
|
||||
}
|
||||
#[cfg(not(unix))]
|
||||
{
|
||||
|
||||
Reference in New Issue
Block a user