fix(webdav): make PATCH's concurrency guard a real compare-and-swap

The app-level ETag re-check before the write still left a gap between the check and the actual UPDATE for a concurrent writer to land in.
Push the check into the write path itself: swap_blob_hash now takes an expected_hash and only applies the SET under the same FOR UPDATE row lock it already held, closing the race instead of just narrowing it. Adds ErrorKind::PreconditionFailed (412) for the CAS-miss path; PUT/WOPI/chunked-upload keep blind-overwrite semantics by passing None
This commit is contained in:
M.Schmidt
2026-07-15 10:50:48 +02:00
parent d57f7bfe3a
commit af74c94028
12 changed files with 157 additions and 78 deletions
+11
View File
@@ -99,6 +99,16 @@ pub trait FileUploadUseCase: Send + Sync + 'static {
/// on the overwrite branch and `authz.require(caller, Create,
/// Folder|Drive(id))` on the new-file branch. Handlers just plumb
/// `caller_id` through — no protocol-layer authz.
///
/// `expected_hash`: forwarded to
/// `FileWritePort::update_file_content_with_blob` on the overwrite
/// branch for compare-and-swap; ignored on the new-file branch
/// (nothing to compare against). Pass `None` for plain PUT/WOPI/
/// chunked-upload last-write-wins semantics; pass the pre-write
/// snapshot's content hash for PATCH, where a concurrent write
/// during the (potentially slow) splice must be rejected rather
/// than silently clobbered.
#[allow(clippy::too_many_arguments)]
async fn update_file_streaming_with_perms(
&self,
path: &str,
@@ -107,6 +117,7 @@ pub trait FileUploadUseCase: Send + Sync + 'static {
content_type: &str,
modified_at: Option<i64>,
caller_id: Uuid,
expected_hash: Option<&str>,
) -> Result<FileDto, DomainError>;
}
+9
View File
@@ -299,6 +299,14 @@ pub trait FileWritePort: Send + Sync + 'static {
///
/// `caller_id` is stamped into `updated_by` alongside the
/// `updated_at` bump (§14 provenance).
///
/// `expected_hash`: when `Some`, makes this a true compare-and-swap —
/// the write only takes effect if the row's current `blob_hash`
/// still equals it, checked and applied atomically under the same
/// row lock (no gap between check and write for a concurrent writer
/// to land in). A mismatch returns `ErrorKind::PreconditionFailed`
/// and leaves the row untouched. `None` keeps the previous
/// blind-overwrite behaviour (PUT/WOPI/chunked-upload finalize).
async fn update_file_content_with_blob(
&self,
file_id: &str,
@@ -306,6 +314,7 @@ pub trait FileWritePort: Send + Sync + 'static {
size: u64,
modified_at: Option<i64>,
caller_id: Uuid,
expected_hash: Option<&str>,
) -> Result<(String, i64), DomainError>;
/// Registers file metadata WITHOUT writing content to disk (write-behind).
@@ -305,7 +305,7 @@ impl FileUploadService {
let file = file_read.get_file(file_id).await?;
let (new_hash, updated_at) = self
.file_write
.update_file_content_with_blob(file_id, &blob.hash, blob.size, None, caller_id)
.update_file_content_with_blob(file_id, &blob.hash, blob.size, None, caller_id, None)
.await?;
// The file maps to a different blob now — stale cached content must
// never be served for the rest of its TTI window.
@@ -506,6 +506,7 @@ impl FileUploadUseCase for FileUploadService {
/// member and cross-tenant PUT. See
/// `docs/plan/authz_audit/nextcloud.md` and the sibling native
/// `/webdav/*` handler.
#[allow(clippy::too_many_arguments)]
async fn update_file_streaming_with_perms(
&self,
path: &str,
@@ -514,6 +515,7 @@ impl FileUploadUseCase for FileUploadService {
content_type: &str,
modified_at: Option<i64>,
caller_id: Uuid,
expected_hash: Option<&str>,
) -> Result<FileDto, DomainError> {
let Some(authz) = &self.authorization else {
return Err(DomainError::internal_error(
@@ -552,6 +554,7 @@ impl FileUploadUseCase for FileUploadService {
blob.size,
modified_at,
caller_id,
expected_hash,
)
.await?;
// Invalidate content cache — file content has changed.
@@ -589,6 +589,7 @@ impl FileWritePort for MockFileRepository {
_size: u64,
_modified_at: Option<i64>,
_caller_id: Uuid,
_expected_hash: Option<&str>,
) -> std::result::Result<(String, i64), DomainError> {
Ok((String::new(), 0))
}