Write to temporary file first, then persist it under the final name.

- This has two benefits.  First, consumers only see the file once we
    are done writing to it, i.e. they don't see a partial file.

  - Second, we guarantee not to overwrite the file until the operation
    is finished.  Therefore, it is safe to use the same file as input
    and output.

  - Fixes #500.

NEWS

@@ -60,6 +60,11 @@
      if the certificate has the self-signed user ID "Alice
      <alice@example.org>", then `--userid-by-email alice@example.org`
      selects "Alice <alice@example.org>" for revocation.
+   - When writing to a file output, we first write to a temporary
+     file, then rename the file at the end of the operation so that it
+     has its desired name.  There are two benefits: no one sees
+     partially written files, and one can safely use the same file as
+     input and output.
 
 * Changes in 0.41.0
 ** New functionality

src/commands/decrypt.rs

@@ -69,6 +69,10 @@ pub fn dispatch(sq: Sq, command: cli::decrypt::Command) -> Result<()> {
                          session_keys);
     if result.is_err() {
         if let Some(path) = command.output.path() {
+            // Drop output here so that the file is persisted and
+            // can be deleted.
+            drop(output);
+
             if let Err(err) = std::fs::remove_file(path) {
                 weprintln!("Decryption failed, failed to remove \
                             output saved to {}: {}",

src/commands/verify.rs

@@ -46,6 +46,10 @@ pub fn dispatch(sq: Sq, command: cli::verify::Command)
                         &mut output, signatures, signers);
     if result.is_err() {
         if let Some(path) = command.output.path() {
+            // Drop output here so that the file is persisted and
+            // can be deleted.
+            drop(output);
+
             if let Err(err) = std::fs::remove_file(path) {
                 weprintln!("Verification failed, failed to remove \
                             unverified output saved to {}: {}",

src/common/file.rs

@@ -1,12 +1,14 @@
 /// Common file handling support.
 
 use std::{
-    fs::OpenOptions,
     io::{self, Write, stdout},
+    path::{Path, PathBuf},
 };
 
 use anyhow::{Context, Result};
 
+use tempfile::NamedTempFile;
+
 use sequoia_openpgp::{
     self as openpgp,
     armor,
@@ -89,11 +91,7 @@ impl FileOrStdout {
         if let Some(path) = self.path() {
             if !path.exists() || sq.overwrite {
                 Ok(Box::new(
-                    OpenOptions::new()
-                        .write(true)
-                        .truncate(true)
-                        .create(true)
-                        .open(path)
+                    PartFileWriter::create(path)
                         .context("Failed to create output file")?,
                 ))
             } else {
@@ -108,6 +106,98 @@ impl FileOrStdout {
     }
 }
 
+/// A writer that writes to a temporary file first, then persists the
+/// file under the desired name.
+///
+/// This has two benefits.  First, consumers only see the file once we
+/// are done writing to it, i.e. they don't see a partial file.
+///
+/// Second, we guarantee not to overwrite the file until the operation
+/// is finished.  Therefore, it is safe to use the same file as input
+/// and output.
+struct PartFileWriter {
+    path: PathBuf,
+    sink: Option<NamedTempFile>,
+}
+
+impl io::Write for PartFileWriter {
+    fn write(&mut self, buf: &[u8]) -> io::Result<usize> {
+        self.sink()?.write(buf)
+    }
+
+    fn flush(&mut self) -> io::Result<()> {
+        self.sink()?.flush()
+    }
+}
+
+impl Drop for PartFileWriter {
+    fn drop(&mut self) {
+        if let Err(e) = self.persist() {
+            weprintln!(initial_indent = "Error: ", "{}", e);
+            std::process::exit(1);
+        }
+    }
+}
+
+impl PartFileWriter {
+    /// Opens a file for writing.
+    ///
+    /// The file will be created under a different name in the target
+    /// directory, and will only be renamed to `path` once
+    /// [`PartFileWriter::persist`] is called or the object is
+    /// dropped.
+    pub fn create<P: AsRef<Path>>(path: P) -> Result<PartFileWriter> {
+        let path = path.as_ref().to_path_buf();
+        let parent = path.parent()
+            .ok_or(anyhow::anyhow!("cannot write to the root"))?;
+        let file_name = path.file_name()
+            .ok_or(anyhow::anyhow!("cannot write to .."))?;
+
+        let mut sink = tempfile::Builder::new();
+
+        // By default, temporary files are 0x600 on Unix.  But, we
+        // rather want created files to respect umask.
+        platform! {
+            unix => {
+                use std::os::unix::fs::PermissionsExt;
+                let all_read_write =
+                    std::fs::Permissions::from_mode(0o666);
+
+                // The permissions will be masked by the user's umask.
+                sink.permissions(all_read_write);
+            },
+            windows => {
+                // We cannot do the same on Windows.
+            },
+        }
+
+        let sink = sink
+            .prefix(file_name)
+            .suffix(".part")
+            .tempfile_in(parent)?;
+
+        Ok(PartFileWriter {
+            path,
+            sink: Some(sink),
+        })
+    }
+
+    /// Returns a mutable reference to the file, or an error.
+    fn sink(&mut self) -> io::Result<&mut NamedTempFile> {
+        self.sink.as_mut().ok_or(io::Error::new(
+            io::ErrorKind::Other,
+            anyhow::anyhow!("file already persisted")))
+    }
+
+    /// Persists the file under its final name.
+    pub fn persist(&mut self) -> Result<()> {
+        if let Some(file) = self.sink.take() {
+            file.persist(&self.path)?;
+        }
+        Ok(())
+    }
+}
+
 /// A writer that buffers all data, and scans for secret keys on drop.
 ///
 /// This is used to assert that we only write secret keys in places