Skip to content

Commit 2935f9b

Browse files
committed
fix(gui): write config files with owner-only permissions
The built-in editor's save path and the first-run starter config previously used std::fs::write, leaving configs at the default 0644 and leaking repository paths and credentials to other local users. Route both sites through new write_tmp_secure / create_new_config_secure helpers that fchmod to 0o600 on the open fd before any write_all, so sensitive contents never exist on disk at a wider mode — including the stale-tmp overwrite case that OpenOptions::mode silently ignores. Fixes #109.
1 parent 419ce52 commit 2935f9b

4 files changed

Lines changed: 98 additions & 3 deletions

File tree

Cargo.lock

Lines changed: 1 addition & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

crates/vykar-gui/Cargo.toml

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,3 +44,6 @@ objc2-app-kit = { version = "0.3", features = ["NSApplication", "NSResponder", "
4444
[build-dependencies]
4545
slint-build = "1.15"
4646
winresource = "0.1"
47+
48+
[dev-dependencies]
49+
tempfile = { workspace = true }

crates/vykar-gui/src/config_helpers.rs

Lines changed: 93 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,46 @@
1-
use std::path::PathBuf;
1+
use std::io::{self, Write};
2+
use std::path::{Path, PathBuf};
23

34
use vykar_core::app;
45
use vykar_core::config;
56

7+
fn finish_secure_write(mut file: std::fs::File, contents: &[u8]) -> io::Result<()> {
8+
// fchmod before writing so contents never exist at a wider mode.
9+
// apply_mode_fd is a no-op on non-Unix.
10+
vykar_core::platform::fs::apply_mode_fd(&file, 0o600)?;
11+
file.write_all(contents)?;
12+
file.sync_all()
13+
}
14+
15+
/// Create a new config file with owner-only permissions (0o600 on Unix).
16+
/// Fails with AlreadyExists if `path` already exists.
17+
pub(crate) fn create_new_config_secure(path: &Path, contents: &[u8]) -> io::Result<()> {
18+
let mut opts = std::fs::OpenOptions::new();
19+
opts.write(true).create_new(true);
20+
#[cfg(unix)]
21+
{
22+
use std::os::unix::fs::OpenOptionsExt;
23+
opts.mode(0o600);
24+
}
25+
let file = opts.open(path)?;
26+
finish_secure_write(file, contents)
27+
}
28+
29+
/// Overwrite `path` with `contents`, applying owner-only permissions (0o600)
30+
/// via fchmod on the open fd before writing. Tolerates a stale file already
31+
/// present at `path` — intended for tmp files in an atomic-rename flow.
32+
pub(crate) fn write_tmp_secure(path: &Path, contents: &[u8]) -> io::Result<()> {
33+
let mut opts = std::fs::OpenOptions::new();
34+
opts.write(true).create(true).truncate(true);
35+
#[cfg(unix)]
36+
{
37+
use std::os::unix::fs::OpenOptionsExt;
38+
opts.mode(0o600);
39+
}
40+
let file = opts.open(path)?;
41+
finish_secure_write(file, contents)
42+
}
43+
644
/// Load and fully validate a config file: parse YAML, check non-empty, validate schedule.
745
/// Returns the parsed repos or a human-readable error string.
846
pub(crate) fn validate_config(
@@ -73,7 +111,7 @@ pub(crate) fn resolve_or_create_config(
73111
if let Some(parent) = path.parent() {
74112
std::fs::create_dir_all(parent)?;
75113
}
76-
std::fs::write(&path, config::minimal_config_template())?;
114+
create_new_config_secure(&path, config::minimal_config_template().as_bytes())?;
77115
path
78116
} else {
79117
// No user-level path available, fall through to file picker
@@ -106,3 +144,56 @@ pub(crate) fn resolve_or_create_config(
106144
};
107145
Ok(app::RuntimeConfig { source, repos })
108146
}
147+
148+
#[cfg(test)]
149+
mod tests {
150+
use super::*;
151+
152+
#[cfg(unix)]
153+
fn mode_of(path: &Path) -> u32 {
154+
use std::os::unix::fs::PermissionsExt;
155+
std::fs::metadata(path).unwrap().permissions().mode()
156+
}
157+
158+
#[test]
159+
fn write_tmp_secure_overrides_stale_wide_mode() {
160+
let dir = tempfile::tempdir().unwrap();
161+
let tmp_path = dir.path().join("config.yaml.tmp");
162+
163+
std::fs::write(&tmp_path, b"leaked-old").unwrap();
164+
#[cfg(unix)]
165+
{
166+
use std::os::unix::fs::PermissionsExt;
167+
std::fs::set_permissions(&tmp_path, std::fs::Permissions::from_mode(0o644)).unwrap();
168+
}
169+
170+
write_tmp_secure(&tmp_path, b"fresh").unwrap();
171+
172+
#[cfg(unix)]
173+
assert_eq!(mode_of(&tmp_path) & 0o077, 0);
174+
assert_eq!(std::fs::read(&tmp_path).unwrap(), b"fresh");
175+
}
176+
177+
#[test]
178+
fn create_new_config_secure_fresh_mode() {
179+
let dir = tempfile::tempdir().unwrap();
180+
let path = dir.path().join("config.yaml");
181+
182+
create_new_config_secure(&path, b"template").unwrap();
183+
184+
#[cfg(unix)]
185+
assert_eq!(mode_of(&path) & 0o077, 0);
186+
assert_eq!(std::fs::read(&path).unwrap(), b"template");
187+
}
188+
189+
#[test]
190+
fn create_new_config_secure_refuses_to_clobber() {
191+
let dir = tempfile::tempdir().unwrap();
192+
let path = dir.path().join("config.yaml");
193+
std::fs::write(&path, b"original").unwrap();
194+
195+
let err = create_new_config_secure(&path, b"new").unwrap_err();
196+
assert_eq!(err.kind(), io::ErrorKind::AlreadyExists);
197+
assert_eq!(std::fs::read(&path).unwrap(), b"original");
198+
}
199+
}

crates/vykar-gui/src/worker/config_cmds.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ pub(super) fn handle_switch_config(ctx: &mut WorkerContext) {
3535
pub(super) fn handle_save_and_apply_config(ctx: &mut WorkerContext, yaml_text: String) {
3636
let config_path = ctx.config_display_path.clone();
3737
let tmp_path = config_path.with_extension("yaml.tmp");
38-
if let Err(e) = std::fs::write(&tmp_path, &yaml_text) {
38+
if let Err(e) = config_helpers::write_tmp_secure(&tmp_path, yaml_text.as_bytes()) {
3939
let _ = ctx
4040
.ui_tx
4141
.send(UiEvent::ConfigSaveError(format!("Write failed: {e}")));

0 commit comments

Comments
 (0)