Skip to content

Commit c64861c

Browse files
committed
fix(dstackup): create the VMM token file 0600 up front (no chmod window)
1 parent 05025bc commit c64861c

2 files changed

Lines changed: 49 additions & 9 deletions

File tree

dstack/crates/dstack-cli-core/src/fsutil.rs

Lines changed: 45 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -31,9 +31,38 @@ fn sibling(path: &Path, suffix: &str) -> PathBuf {
3131
/// durable across a power loss. `tmp` and `path` are in the same directory so
3232
/// the rename is atomic.
3333
pub fn write_atomic(path: &Path, contents: &str) -> Result<()> {
34+
write_atomic_inner(path, contents, None)
35+
}
36+
37+
/// like [`write_atomic`], but the temp file is created with `mode` (Unix
38+
/// permission bits) *before* any content is written, so a secret never exists
39+
/// on disk with broader-than-intended permissions — not even transiently
40+
/// between the rename and a follow-up `chmod`. Use for credential files
41+
/// (`0o600`). The final file keeps `mode` because `rename` preserves it.
42+
pub fn write_atomic_mode(path: &Path, contents: &str, mode: u32) -> Result<()> {
43+
write_atomic_inner(path, contents, Some(mode))
44+
}
45+
46+
fn write_atomic_inner(path: &Path, contents: &str, mode: Option<u32>) -> Result<()> {
3447
let tmp = sibling(path, ".tmp");
35-
let mut f =
36-
File::create(&tmp).with_context(|| format!("creating temp file {}", tmp.display()))?;
48+
let mut opts = OpenOptions::new();
49+
opts.write(true).create(true).truncate(true);
50+
#[cfg(unix)]
51+
if let Some(mode) = mode {
52+
use std::os::unix::fs::OpenOptionsExt;
53+
opts.mode(mode);
54+
}
55+
let mut f = opts
56+
.open(&tmp)
57+
.with_context(|| format!("creating temp file {}", tmp.display()))?;
58+
// if a stale temp file survived a crash, `create` reused it without
59+
// resetting its mode; tighten it before writing the secret.
60+
#[cfg(unix)]
61+
if let Some(mode) = mode {
62+
use std::os::unix::fs::PermissionsExt;
63+
std::fs::set_permissions(&tmp, std::fs::Permissions::from_mode(mode))
64+
.with_context(|| format!("setting mode on {}", tmp.display()))?;
65+
}
3766
f.write_all(contents.as_bytes())
3867
.with_context(|| format!("writing {}", tmp.display()))?;
3968
f.sync_all()
@@ -86,6 +115,20 @@ mod tests {
86115
let _ = std::fs::remove_dir_all(&dir);
87116
}
88117

118+
#[cfg(unix)]
119+
#[test]
120+
fn atomic_write_mode_creates_owner_only_file() {
121+
use std::os::unix::fs::PermissionsExt;
122+
let dir = std::env::temp_dir().join(format!("dstack-fsmode-{}", std::process::id()));
123+
std::fs::create_dir_all(&dir).unwrap();
124+
let p = dir.join("token");
125+
write_atomic_mode(&p, "secret", 0o600).unwrap();
126+
assert_eq!(std::fs::read_to_string(&p).unwrap(), "secret");
127+
let mode = std::fs::metadata(&p).unwrap().permissions().mode() & 0o777;
128+
assert_eq!(mode, 0o600, "credential file must be 0600, got {mode:o}");
129+
let _ = std::fs::remove_dir_all(&dir);
130+
}
131+
89132
#[test]
90133
fn lock_is_reentrant_within_process_after_drop() {
91134
let dir = std::env::temp_dir().join(format!("dstack-fslock-{}", std::process::id()));

dstack/crates/dstackup/src/install.rs

Lines changed: 4 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1230,14 +1230,11 @@ pub(crate) fn read_token_file(path: &Path) -> Option<String> {
12301230
}
12311231

12321232
/// write the management-API token atomically with owner-only (0600)
1233-
/// permissions — it is a bearer credential.
1233+
/// permissions — it is a bearer credential, so it is created 0600 up front
1234+
/// (never exposed with wider bits, even transiently).
12341235
fn write_token_file(path: &Path, token: &str) -> Result<()> {
1235-
use std::os::unix::fs::PermissionsExt;
1236-
dstack_cli_core::fsutil::write_atomic(path, token)
1237-
.with_context(|| format!("writing {}", path.display()))?;
1238-
fs::set_permissions(path, fs::Permissions::from_mode(0o600))
1239-
.with_context(|| format!("setting 0600 on {}", path.display()))?;
1240-
Ok(())
1236+
dstack_cli_core::fsutil::write_atomic_mode(path, token, 0o600)
1237+
.with_context(|| format!("writing {}", path.display()))
12411238
}
12421239

12431240
#[cfg(test)]

0 commit comments

Comments
 (0)