From 7af3e66ce98ee6752edf87521d150f38a9781287 Mon Sep 17 00:00:00 2001
From: marshmallow <git@althaea.zone>
Date: Sat, 2 May 2026 16:38:35 +1000
Subject: [PATCH] use OpenOptions instead of manually setting permissions
 (#481)

---
 CHANGELOG.md                 |  1 +
 Cargo.toml                   |  2 +-
 crates/key_agent/src/main.rs | 76 ++++++++++++++++++++++++++----------
 3 files changed, 57 insertions(+), 22 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 0f65926..4fddf97 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -26,6 +26,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - Fixed garnix docs links in documentation.
 - Forces `bash` instead of remote user's potentially unsupported shell. This bug
   was causing strange and hard to diagnose issues.
+- Fixed a possible time-of-check to time-of-use bug while setting key permissions.
 
 ## [v1.2.0] - 2026-03-18
 
diff --git a/Cargo.toml b/Cargo.toml
index e6671d3..65bf19d 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -27,7 +27,7 @@ tracing-subscriber = "0.3.20"
 im = { version = "15.1.0", features = ["serde"] }
 anyhow = "1.0.100"
 prost = "0.14.1"
-nix = { version = "0.31.0", features = ["user", "poll", "term"] }
+nix = { version = "0.31.0", features = ["user", "poll", "term", "fs"] }
 miette = { version = "7.6.0", features = ["fancy"] }
 thiserror = "2.0.17"
 sha2 = "0.11.0"
diff --git a/crates/key_agent/src/main.rs b/crates/key_agent/src/main.rs
index faab622..a316da0 100644
--- a/crates/key_agent/src/main.rs
+++ b/crates/key_agent/src/main.rs
@@ -2,17 +2,19 @@
 // Copyright 2024-2025 wire Contributors
 
 #![deny(clippy::pedantic)]
+use anyhow::Context;
 use base64::Engine;
 use base64::prelude::BASE64_STANDARD;
 use futures_util::stream::StreamExt;
+use nix::sys::stat::fchmod;
+use nix::unistd::fchown;
 use nix::unistd::{Group, User};
 use prost::Message;
 use prost::bytes::Bytes;
 use sha2::{Digest, Sha256};
-use std::os::unix::fs::PermissionsExt;
-use std::os::unix::fs::chown;
+use std::os::fd::AsFd;
 use std::path::{Path, PathBuf};
-use tokio::fs::File;
+use tokio::fs::OpenOptions;
 use tokio::io::AsyncWriteExt;
 use tokio_util::codec::{FramedRead, LengthDelimitedCodec};
 use wire_key_agent::keys::KeySpec;
@@ -63,26 +65,58 @@ async fn main() -> Result<(), anyhow::Error> {
         }
 
         let path = PathBuf::from(&spec.destination);
-        create_path(&path)?;
-
-        let mut file = File::create(path).await?;
-        let mut permissions = file.metadata().await?.permissions();
-
-        permissions.set_mode(spec.unix_mode);
-        file.set_permissions(permissions).await?;
-
-        let user = User::from_name(&spec.user)?;
-        let group = Group::from_name(&spec.group)?;
+        create_path(&path).context("creating directory for key")?;
+
+        let mut file = OpenOptions::new()
+            .write(true)
+            .create(true)
+            .truncate(true)
+            // only applies if the file is created
+            .mode(spec.unix_mode)
+            .custom_flags(nix::libc::O_NOFOLLOW)
+            .open(&path)
+            .await
+            .context("opening file")?;
+
+        // enforce permission on existing files
+        let mode = nix::sys::stat::Mode::from_bits(spec.unix_mode)
+            .with_context(|| format!("failed to create unix mode: {:o}", spec.unix_mode))?;
+
+        fchmod(file.as_fd(), mode)
+            .with_context(|| format!("setting permissions of fd to {:o}", spec.unix_mode))?;
+
+        // Default uid/gid to 0. This is then wrapped around an Option again for
+        // the function.
+        let user = Some(
+            User::from_name(&spec.user)
+                .context("obtaining user")?
+                .map_or_else(
+                    || {
+                        println!("warning: defaulting uid to `0`");
+
+                        0.into()
+                    },
+                    |user| user.uid,
+                ),
+        );
+        let group = Some(
+            Group::from_name(&spec.group)
+                .context("obtaining group")?
+                .map_or_else(
+                    || {
+                        println!("warning: defaulting gid to `0`");
+
+                        0.into()
+                    },
+                    |group| group.gid,
+                ),
+        );
 
-        chown(
-            spec.destination,
-            // Default uid/gid to 0. This is then wrapped around an Option again for
-            // the function.
-            Some(user.map_or(0, |user| user.uid.into())),
-            Some(group.map_or(0, |group| group.gid.into())),
-        )?;
+        // set permission on new files
+        fchown(&file, user, group)
+            .with_context(|| format!("setting ownership of fd to {user:?}, {group:?}"))?;
 
-        file.write_all(&key_bytes).await?;
+        file.write_all(&key_bytes).await.context("writing to fd")?;
 
         // last key, goobye
         if spec.last {
-- 
2.51.2