Skip to content

feat(objectstore): S3 conditional writes to prevent silent overwrites#62473

Open
solracsf wants to merge 1 commit into
masterfrom
feat/objectstore-conditional-writes
Open

feat(objectstore): S3 conditional writes to prevent silent overwrites#62473
solracsf wants to merge 1 commit into
masterfrom
feat/objectstore-conditional-writes

Conversation

@solracsf

Copy link
Copy Markdown
Member

Summary

Nextcloud stores each file's data as an object keyed by the file-cache auto-increment id (urn:oid:<fileid>), so a newly created file always targets an object key that should not exist yet. When the file cache and the bucket drift out of sync, a DB restored from backup, two instances pointed at one bucket, or duplicated file ids, the current unconditional write
silently overwrites and destroys the existing object.

This adds opt-in S3 conditional writes: when enabled, creates send If-None-Match: * so the store atomically refuses to overwrite an existing object, turning silent data loss into a loud, data-preserving failure.

https://docs.aws.amazon.com/AmazonS3/latest/userguide/conditional-writes.html

  • S3 backend: If-None-Match: * on single-PUT and multipart creates; 412 typed exception, bounded 409 retry, abort-safe multipart completion.
  • ObjectStoreStorage::writeStream uses it only for newly created files; on refusal it cleans up and logs a critical, actionable message (no data overwritten).
  • New per-bucket conditional_writes config: false (default), 'auto' (one cached bucket probe, enable only if enforced), true. Self-disables on SigV2.

Compatibility

  • Opt-in: default false, so upgrades are unchanged.
  • Overwrites and the disabled path emit byte-identical S3 requests to before.
  • No DB schema change.

Testing

  • New unit tests: storage-layer routing + refusal-without-data-loss; S3 single/ multipart refusal + data preservation + config-off + cached-probe.
  • Verified against MinIO

Checklist

AI (if applicable)

  • The content of this PR was reviewed, and some tests written, by Fable

@solracsf solracsf added this to the Nextcloud 35 milestone Jul 24, 2026
@solracsf
solracsf requested a review from a team as a code owner July 24, 2026 08:38
@solracsf
solracsf requested review from Altahrim and salmart-dev and removed request for a team July 24, 2026 08:38
@solracsf solracsf added the 3. to review Waiting for reviews label Jul 24, 2026
@solracsf
solracsf requested review from come-nc and leftybournes July 24, 2026 08:38
@solracsf solracsf added feature: object storage community pull requests from community labels Jul 24, 2026
@solracsf
solracsf requested a review from icewind1991 July 24, 2026 08:40
@solracsf solracsf self-assigned this Jul 24, 2026
@solracsf
solracsf force-pushed the feat/objectstore-conditional-writes branch from d71a875 to 9beefe3 Compare July 24, 2026 09:48
@solracsf

Copy link
Copy Markdown
Member Author
  1. Tests\Core\Sharing\Recipient\TeamShareRecipientTypeTest
    Circles app is not installed

PHPunit failure does not seem related.

Object keys are derived from the file-cache auto-increment id, so a newly
created file always targets a key that must not exist yet. When the cache and
the bucket drift apart - a database restored from backup, two instances sharing
one bucket, duplicated file ids - the current unconditional write silently
overwrites and destroys the existing object.

Send "If-None-Match: *" for newly created files so the store refuses the write
atomically, turning silent data loss into a loud, data-preserving failure.

- S3: If-None-Match on single-PUT and multipart creates, typed 412 exception,
  bounded 409 retry with backoff, abort-safe multipart completion.
- ObjectStoreStorage::writeStream uses it only for creates; on refusal it
  removes the cache entry and logs an actionable critical message.
- Per-bucket "conditional_writes" config: false (default), 'auto' (one cached
  bucket probe, enable only if enforced) or true. Self-disables on SigV2;
  an unrecognized value is reported and treated as false.

Overwrites and the disabled path emit byte-identical S3 requests to before, and
there is no DB schema change. Chunked uploads create their target object before
filling it and therefore cannot use the header; they keep the previous
behaviour, as documented in config.sample.php.

Assisted-by: ClaudeCode:claude-opus-5
Signed-off-by: Git'Fellow <12234510+solracsf@users.noreply.github.com>
@solracsf
solracsf force-pushed the feat/objectstore-conditional-writes branch from 9beefe3 to d577cf3 Compare July 25, 2026 14:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

3. to review Waiting for reviews AI assisted community pull requests from community feature: object storage

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants