From f47106328d7bf9f85129ac1702c8639749b29593 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Bern=C3=A1t=20G=C3=A1bor?= <gaborjbernat@gmail.com>
Date: Fri, 17 Jul 2026 21:19:46 -0700
Subject: [PATCH] =?UTF-8?q?=F0=9F=A7=AA=20test(storage):=20de-flake=20S3?=
 =?UTF-8?q?=20stage-error=20tests?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

The S3 begin- and commit-failure tests forced their errors with permission
bits: a read-only staging directory and a chmod-000 stage. Root ignores
those bits, so on a root runner begin and commit succeed and unwrap_err
panics. The commit test also restored the stage's permissions after
commit, racing the asynchronous drop that removes the stage and panicking
with a not-found when the drop won.

Trigger the same failures through conditions no user can bypass and with no
cleanup to race: point the staging path at a regular file so creating the
staging directory fails as not-a-directory, and remove the local stage
before commit reads it so the upload's read fails as not-found. The tests
hold on non-root, root, and under coverage instrumentation, and still cover
the staging and read failures.
---
 crates/peryx-storage/tests/s3_backend.rs | 33 +++++++++++-------------
 1 file changed, 15 insertions(+), 18 deletions(-)

diff --git a/crates/peryx-storage/tests/s3_backend.rs b/crates/peryx-storage/tests/s3_backend.rs
index dae621c3..a9e90db1 100644
--- a/crates/peryx-storage/tests/s3_backend.rs
+++ b/crates/peryx-storage/tests/s3_backend.rs
@@ -350,32 +350,29 @@ async fn test_s3_read_operations_surface_a_server_error() {
     assert_eq!(storage.delete(&digest).await.unwrap_err().kind(), BlobErrorKind::Io);
 }
 
-#[cfg(unix)]
 #[tokio::test]
 async fn test_s3_begin_surfaces_a_staging_failure() {
-    use std::os::unix::fs::PermissionsExt as _;
     let (server, _mock) = mounted().await;
-    let staging = tempfile::tempdir().unwrap();
+    // The staging path is a regular file, so creating the staging directory fails with a
+    // not-a-directory error that no user, root included, can bypass.
+    let root = tempfile::tempdir().unwrap();
+    let staging = root.path().join("staging");
+    std::fs::write(&staging, b"regular file, not a directory").unwrap();
     let config = S3Config::new(settings(server.uri())).unwrap();
-    let storage = BlobStorage::s3(config, credentials(), staging.path().to_path_buf());
-    std::fs::set_permissions(staging.path(), std::fs::Permissions::from_mode(0o555)).unwrap();
-    let error = storage.begin().await.map(|_| ()).unwrap_err();
-    std::fs::set_permissions(staging.path(), std::fs::Permissions::from_mode(0o755)).unwrap();
-    assert_eq!(error.kind(), BlobErrorKind::Io);
+    let storage = BlobStorage::s3(config, credentials(), staging);
+    assert_eq!(storage.begin().await.map(|_| ()).unwrap_err().kind(), BlobErrorKind::Io);
 }
 
-#[cfg(unix)]
 #[tokio::test]
-async fn test_s3_commit_surfaces_an_unreadable_stage() {
-    use std::os::unix::fs::PermissionsExt as _;
+async fn test_s3_commit_surfaces_a_missing_stage() {
     let (server, _mock) = mounted().await;
-    let (storage, _staging) = storage(&server, |settings| settings.max_retries = 0);
-    let staged = storage.stage_bytes(b"unreadable").await.unwrap();
-    let path = staged.with_materialized(std::path::Path::to_path_buf);
-    std::fs::set_permissions(&path, std::fs::Permissions::from_mode(0o000)).unwrap();
-    let error = staged.commit().await.unwrap_err();
-    std::fs::set_permissions(&path, std::fs::Permissions::from_mode(0o644)).unwrap();
-    assert_eq!(error.kind(), BlobErrorKind::Io);
+    let (storage, _staging) = storage(&server, |_| {});
+    let staged = storage.stage_bytes(b"vanishes").await.unwrap();
+    // Remove the local stage before commit reads it to upload. A missing file fails the read for any
+    // user, unlike a permission bit that root ignores, and there is no permission to restore that
+    // could race the stage's asynchronous drop.
+    std::fs::remove_file(staged.with_materialized(std::path::Path::to_path_buf)).unwrap();
+    assert_eq!(staged.commit().await.unwrap_err().kind(), BlobErrorKind::Io);
 }
 
 #[tokio::test]