feat(by-hash): allow /by-hash even if blob is trashed

- permit reuse of blob where file is trashed (by-hash and chunked) - add hurl test on /api/files/by-hash - add anti enumeration of blob (404 is always blob_not_owned_by_caller)
2026-06-16 22:29:51 +02:00
parent 9519c1fee2
commit 68abb891d7
5 changed files with 350 additions and 13 deletions
@@ -508,9 +508,11 @@ impl DedupService {
    // that from becoming a content oracle or a poisoning vector:
    //
    // 1. **Ownership**: without bytes, a caller may only claim chunks that
-    //    are already reachable through their own (non-trashed) files, or
-    //    unreferenced orphans (ref_count = 0 — i.e. "I just uploaded it").
-    //    Everything else must be uploaded; the store dedups it on write.
+    //    are already reachable through their own files (live OR trashed,
+    //    since trash is a deferred-delete state — the user can restore the
+    //    file at any time, so the content is still theirs), or unreferenced
+    //    orphans (ref_count = 0 — i.e. "I just uploaded it"). Everything
+    //    else must be uploaded; the store dedups it on write.
    // 2. **Verification**: a declared file_hash is never trusted — the
    //    commit re-reads the proposed chunk sequence server-side and
    //    recomputes BLAKE3 before any manifest row exists. A forged hash
@@ -589,8 +591,16 @@ impl DedupService {

    /// Of `hashes` (distinct), the subset `caller_id` may claim without
    /// uploading bytes: chunks referenced by manifests of the caller's
-    /// non-trashed files, or directly referenced as (legacy) whole-file
-    /// blobs. Backed by the GIN index on `chunk_manifests.chunk_hashes`.
+    /// files (live or trashed), or directly referenced as (legacy)
+    /// whole-file blobs. Backed by the GIN index on
+    /// `chunk_manifests.chunk_hashes`.
+    ///
+    /// Trashed files count as ownership: a trashed file's content is still
+    /// the caller's (restorable until trash-empty), so a re-upload of the
+    /// same content should hit the dedup fast path instead of forcing the
+    /// caller to re-send bytes they already have on the server. Must stay
+    /// in lockstep with [`pin_claimable_chunks`], which actually bumps the
+    /// ref_count using the same entitlement set.
    pub async fn claimable_chunks(
        &self,
        caller_id: uuid::Uuid,
@@ -605,12 +615,12 @@ impl DedupService {
                        SELECT 1
                          FROM storage.files f
                          JOIN storage.chunk_manifests m ON m.file_hash = f.blob_hash
-                         WHERE f.user_id = $2 AND NOT f.is_trashed
+                         WHERE f.user_id = $2
                           AND m.chunk_hashes @> ARRAY[c.h]
                    )
                 OR EXISTS (
                        SELECT 1 FROM storage.files f2
-                         WHERE f2.user_id = $2 AND NOT f2.is_trashed
+                         WHERE f2.user_id = $2
                           AND f2.blob_hash = c.h
                    )",
        )
@@ -629,6 +639,12 @@ impl DedupService {
    /// concurrent last-reference delete can never be resurrected and a
    /// non-entitled hash is simply not returned.
    ///
+    /// Entitlement includes files in trash: a trashed file is still owned
+    /// by the user, the content is still theirs to re-reference, and the
+    /// race with trash-empty is handled the same way as `add_reference` —
+    /// if GC has already deleted the blob row, the UPDATE affects 0 rows
+    /// and the hash is simply absent from the returned set.
+    ///
    /// Returns the set actually pinned; the caller compares against its
    /// input and reports the difference as `still_missing`.
    pub async fn pin_claimable_chunks(
@@ -648,12 +664,12 @@ impl DedupService {
                             SELECT 1
                               FROM storage.files f
                               JOIN storage.chunk_manifests m ON m.file_hash = f.blob_hash
-                              WHERE f.user_id = $2 AND NOT f.is_trashed
+                              WHERE f.user_id = $2
                                AND m.chunk_hashes @> ARRAY[b.hash::text]
                         )
                      OR EXISTS (
                             SELECT 1 FROM storage.files f2
-                              WHERE f2.user_id = $2 AND NOT f2.is_trashed
+                              WHERE f2.user_id = $2
                                AND f2.blob_hash = b.hash
                         ) )
              RETURNING b.hash",
@@ -1034,11 +1050,11 @@ impl DedupService {
            .unwrap_or(false)
    }

-    /// Returns `true` if `user_id` owns at least one (non-trashed) file that
+    /// Returns `true` if `user_id` owns at least one (even trashed) file that
    /// references the blob identified by `hash`.
    pub async fn user_owns_blob_reference(&self, hash: &str, user_id: &str) -> bool {
        sqlx::query_scalar::<_, bool>(
-            "SELECT EXISTS(SELECT 1 FROM storage.files WHERE blob_hash = $1 AND user_id = $2::uuid AND NOT is_trashed)",
+            "SELECT EXISTS(SELECT 1 FROM storage.files WHERE blob_hash = $1 AND user_id = $2::uuid)",
        )
        .bind(hash)
        .bind(user_id)
@@ -116,7 +116,31 @@ impl FileHandler {
            .await
        {
            Ok(file) => Self::created_json_response(&file).into_response(),
-            Err(err) => Self::domain_error_response(err).into_response(),
+            Err(err) => {
+                // Anti-enumeration shape: every "caller cannot reach this
+                // hash" outcome collapses into the same 404 with an
+                // `upload_path` hint, regardless of whether the hash exists
+                // globally, is owned by another tenant, or got GC'd in a
+                // race against trash-empty. Hides the cross-tenant content
+                // existence oracle and tells the client where to fall back.
+                //
+                // Three NotFound("Blob", _) paths in the service map here:
+                //   1. user_owns_blob_reference returned false
+                //   2. get_blob_metadata returned None (blob row vanished)
+                //   3. add_reference lost the race with GC (rows_affected==0)
+                use crate::common::errors::ErrorKind;
+                if err.kind == ErrorKind::NotFound && err.entity_type == "Blob" {
+                    return Response::builder()
+                        .status(StatusCode::NOT_FOUND)
+                        .header(header::CONTENT_TYPE, "application/json")
+                        .body(Body::from(
+                            r#"{"error":"blob_not_owned_by_caller","upload_path":"/api/files/upload"}"#,
+                        ))
+                        .unwrap()
+                        .into_response();
+                }
+                Self::domain_error_response(err).into_response()
+            }
        }
    }

@@ -0,0 +1,295 @@
+# =============================================================
+# OxiCloud — Upload precheck (`POST /api/files/by-hash`)
+# =============================================================
+# Validates the content_hash precheck that lets a client skip a
+# body upload when the server already has the content for THIS
+# caller. The request shape mirrors `FileDto.content_hash` — the
+# same 64-char lowercase BLAKE3 hex digest the server returned
+# on a previous upload.
+#
+# Scope-critical security rule (server-enforced): the precheck
+# returns 201 only when the calling user already owns at least
+# one file with the same BLAKE3 hash. Cross-user matches are
+# invisible — hash probing cannot leak other tenants' content.
+#
+# Coverage:
+#   1. Upload hello.txt normally → 201, content_hash captured
+#   2. Precheck with the matching content_hash → 201, new
+#      file_id but SAME content_hash (dedup hit)
+#   3. Precheck with a fake/random hash → 404
+#   4. Malformed content_hash (non-hex chars) → 400
+#   5. Wrong-length content_hash → 400
+#
+# Fixture: tests/fixtures/hello-copy.txt (32 bytes; same content as
+# hello.txt). We seed with hello-copy.txt — NOT hello.txt — because
+# several earlier tests in run.sh (trash, chunked_upload_cap, …)
+# upload hello.txt into the home folder and don't reliably purge it,
+# which would collide with our seed step here (409). The filename
+# `hello-copy.txt` is unique to this test.
+#   BLAKE3 hex:    b2208c5dc33ff951227bd0c139f5eccb04105d6da6a7519ee23f7bc00a17bb5a
+# =============================================================
+
+
+# ─────────────────────────────────────────────────────────────
+# Step 1 — Login
+# ─────────────────────────────────────────────────────────────
+POST {{base_url}}/api/auth/login
+Content-Type: application/json
+{
+  "username": "{{username}}",
+  "password": "{{password}}"
+}
+
+HTTP 200
+[Captures]
+token: jsonpath "$.access_token"
+
+
+# ─────────────────────────────────────────────────────────────
+# Step 2 — Resolve home folder
+# ─────────────────────────────────────────────────────────────
+GET {{base_url}}/api/folders
+Authorization: Bearer {{token}}
+
+HTTP 200
+[Captures]
+home_folder_id: jsonpath "$[0].id"
+
+
+# ─────────────────────────────────────────────────────────────
+# Step 3 — Seed: upload hello-copy.txt via the normal multipart
+#          path. Captures the BLAKE3 server-side `content_hash`
+#          that the precheck step then matches against.
+# ─────────────────────────────────────────────────────────────
+POST {{base_url}}/api/files/upload
+Authorization: Bearer {{token}}
+[MultipartFormData]
+folder_id: {{home_folder_id}}
+file: file,fixtures/hello-copy.txt; text/plain
+
+HTTP 201
+[Captures]
+seed_file_id: jsonpath "$.id"
+seed_content_hash: jsonpath "$.content_hash"
+[Asserts]
+jsonpath "$.name" == "hello-copy.txt"
+jsonpath "$.size" == 32
+jsonpath "$.content_hash" == "b2208c5dc33ff951227bd0c139f5eccb04105d6da6a7519ee23f7bc00a17bb5a"
+
+
+# ─────────────────────────────────────────────────────────────
+# Step 4 — Duplicate-name probe (incidental but useful): try
+#          uploading the same fixture again, same name, same
+#          folder → 409 Conflict. This catches the (folder_id,
+#          name, user_id) WHERE NOT is_trashed unique index in
+#          `storage.files`.
+# ─────────────────────────────────────────────────────────────
+POST {{base_url}}/api/files/upload
+Authorization: Bearer {{token}}
+[MultipartFormData]
+folder_id: {{home_folder_id}}
+file: file,fixtures/hello-copy.txt; text/plain
+
+HTTP 409
+
+
+# ─────────────────────────────────────────────────────────────
+# Step 5 — Precheck HIT: caller now owns the BLAKE3 of
+#          hello-copy.txt, so by-hash with that hash must
+#          201 and return a NEW file_id (the metadata row is
+#          new — the blob is shared).
+#
+#          Expectations:
+#          - status: 201
+#          - file_id differs from the seed
+#          - content_hash matches the seed (proves dedup hit)
+#          - filename is the one we requested
+# ─────────────────────────────────────────────────────────────
+POST {{base_url}}/api/files/by-hash
+Authorization: Bearer {{token}}
+Content-Type: application/json
+{
+  "name": "hello-precheck-bypass.txt",
+  "folder_id": "{{home_folder_id}}",
+  "hash": "b2208c5dc33ff951227bd0c139f5eccb04105d6da6a7519ee23f7bc00a17bb5a"
+}
+
+HTTP 201
+[Captures]
+copy_file_id: jsonpath "$.id"
+copy_content_hash: jsonpath "$.content_hash"
+[Asserts]
+jsonpath "$.name" == "hello-precheck-bypass.txt"
+jsonpath "$.size" == 32
+jsonpath "$.content_hash" == "b2208c5dc33ff951227bd0c139f5eccb04105d6da6a7519ee23f7bc00a17bb5a"
+
+
+# ─────────────────────────────────────────────────────────────
+# Step 6 — Same hash, different filename, in a NEW request →
+#          should hit again. Confirms the precheck isn't a
+#          one-shot bound to the seed file.
+# ─────────────────────────────────────────────────────────────
+POST {{base_url}}/api/files/by-hash
+Authorization: Bearer {{token}}
+Content-Type: application/json
+{
+  "name": "hello-precheck-bypass-2.txt",
+  "folder_id": "{{home_folder_id}}",
+  "hash": "b2208c5dc33ff951227bd0c139f5eccb04105d6da6a7519ee23f7bc00a17bb5a"
+}
+
+HTTP 201
+[Asserts]
+jsonpath "$.name" == "hello-precheck-bypass-2.txt"
+jsonpath "$.content_hash" == "b2208c5dc33ff951227bd0c139f5eccb04105d6da6a7519ee23f7bc00a17bb5a"
+
+
+# ─────────────────────────────────────────────────────────────
+# Step 7 — Precheck MISS with a random/never-uploaded hash →
+#          404 with `error: blob_not_owned_by_caller` and an
+#          `upload_path` hint. The 404 shape is the
+#          anti-enumeration boundary: same response whether the
+#          hash doesn't exist anywhere or exists but is owned
+#          by another user.
+#
+#          The fake hash below is just 32 bytes of 0xFF (well-
+#          formed BLAKE3 shape, vanishingly unlikely to collide).
+# ─────────────────────────────────────────────────────────────
+POST {{base_url}}/api/files/by-hash
+Authorization: Bearer {{token}}
+Content-Type: application/json
+{
+  "name": "should-not-create.bin",
+  "folder_id": "{{home_folder_id}}",
+  "hash": "ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff"
+}
+
+HTTP 404
+[Asserts]
+jsonpath "$.error" == "blob_not_owned_by_caller"
+jsonpath "$.upload_path" == "/api/files/upload"
+
+
+# ─────────────────────────────────────────────────────────────
+# Step 8 — Malformed content_hash (non-hex chars) → 400.
+#          Defensive parse: typos shouldn't silently disable
+#          the integrity check.
+# ─────────────────────────────────────────────────────────────
+POST {{base_url}}/api/files/by-hash
+Authorization: Bearer {{token}}
+Content-Type: application/json
+{
+  "name": "should-not-create.bin",
+  "folder_id": "{{home_folder_id}}",
+  "hash": "this-is-not-hex-content-hash-zzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzz"
+}
+
+HTTP 400
+
+
+# ─────────────────────────────────────────────────────────────
+# Step 9 — Wrong-length content_hash → 400. A 64-char string
+#          is required (32-byte BLAKE3); anything else (here a
+#          SHA-1-sized 40-char hex) is rejected at the input
+#          boundary before the lookup happens.
+# ─────────────────────────────────────────────────────────────
+POST {{base_url}}/api/files/by-hash
+Authorization: Bearer {{token}}
+Content-Type: application/json
+{
+  "name": "should-not-create.bin",
+  "folder_id": "{{home_folder_id}}",
+  "hash": "a94a8fef8c5d539e7e0f9c4b4d2c93e88f44f3f8"
+}
+
+HTTP 400
+
+
+# ─────────────────────────────────────────────────────────────
+# Step 10 — Trashed-blob precheck setup. Upload a fixture whose
+#           BLAKE3 is GUARANTEED unique to this test (the
+#           `hello-trashed.txt` fixture isn't referenced anywhere
+#           else under tests/), so the only file owning the blob
+#           after step 11 will be the one we trash.
+#
+#           Captures `trashed_seed_id` + `trashed_seed_hash`
+#           which steps 11 and 12 consume.
+# ─────────────────────────────────────────────────────────────
+POST {{base_url}}/api/files/upload
+Authorization: Bearer {{token}}
+[MultipartFormData]
+folder_id: {{home_folder_id}}
+file: file,fixtures/hello-trashed.txt; text/plain
+
+HTTP 201
+[Captures]
+trashed_seed_id: jsonpath "$.id"
+trashed_seed_hash: jsonpath "$.content_hash"
+[Asserts]
+jsonpath "$.name" == "hello-trashed.txt"
+jsonpath "$.size" == 35
+
+
+# ─────────────────────────────────────────────────────────────
+# Step 11 — Trash the seed (soft-delete). The blob's ref_count
+#           stays > 0 because the file row still exists with
+#           is_trashed = true. Pre-change behaviour would have
+#           hidden the blob from `/api/files/by-hash`; the new
+#           behaviour treats trashed-file ownership as ownership.
+# ─────────────────────────────────────────────────────────────
+DELETE {{base_url}}/api/files/{{trashed_seed_id}}
+Authorization: Bearer {{token}}
+
+# Either 200 or 204 depending on whether the server returns a
+# body. The trash service uses soft-delete (UPDATE is_trashed = true),
+# not a hard delete, so the blob row is untouched.
+HTTP *
+
+
+# ─────────────────────────────────────────────────────────────
+# Step 12 — Precheck HIT on a trashed-only owner. Must succeed
+#           (201) and return a NEW live file pointing at the same
+#           blob — even though the only OTHER reference to the
+#           blob is a trashed file. Verifies the dropped
+#           `AND NOT f.is_trashed` filter in
+#           `user_owns_blob_reference` and `pin_claimable_chunks`.
+# ─────────────────────────────────────────────────────────────
+POST {{base_url}}/api/files/by-hash
+Authorization: Bearer {{token}}
+Content-Type: application/json
+{
+  "name": "restored-via-by-hash.txt",
+  "folder_id": "{{home_folder_id}}",
+  "hash": "{{trashed_seed_hash}}"
+}
+
+HTTP 201
+[Captures]
+trashed_copy_id: jsonpath "$.id"
+[Asserts]
+jsonpath "$.name" == "restored-via-by-hash.txt"
+jsonpath "$.size" == 35
+jsonpath "$.content_hash" == "{{trashed_seed_hash}}"
+
+
+# ─────────────────────────────────────────────────────────────
+# Step 13 — Cleanup. Trash every file created by this script;
+#           the next run starts clean. Seed (step 4) + copies
+#           (steps 5+6) share one blob; trashed seed (step 10)
+#           + by-hash restore (step 12) share another. Trashing
+#           all four lets the GC ref-count both blobs to zero.
+# ─────────────────────────────────────────────────────────────
+DELETE {{base_url}}/api/files/{{seed_file_id}}
+Authorization: Bearer {{token}}
+
+HTTP *
+
+DELETE {{base_url}}/api/files/{{copy_file_id}}
+Authorization: Bearer {{token}}
+
+HTTP *
+
+DELETE {{base_url}}/api/files/{{trashed_copy_id}}
+Authorization: Bearer {{token}}
+
+HTTP *
@@ -155,7 +155,8 @@ hurl --variables-file "$API_DIR/test.env" --file-root "$REPO_ROOT/tests" --test
  "$API_DIR/nc_admin_views_other_user.hurl" \
  "$API_DIR/admin_user_ops.hurl" \
  "$API_DIR/chunked_upload_cap.hurl" \
-  "$API_DIR/nc_auth_failures.hurl"
+  "$API_DIR/nc_auth_failures.hurl" \
+  "$API_DIR/dedup_create.hurl"

 #bash "$API_DIR/dedup_bulk_upload.sh"

@@ -0,0 +1 @@
+hello trashed by-hash test fixture