From dd9e3b8868f02b984c08ee5f2ab590c52a74242d Mon Sep 17 00:00:00 2001 From: Edouard Vanbelle Date: Fri, 19 Jun 2026 12:12:18 +0200 Subject: [PATCH] feat(drive): test Tantivy lookup with drive ensure that a user that don't havee permission to a drive cannot search elements in this drive note: current design: Index are associated to drive, so if a document / directory is shared, the shared resource will not be in index for targetted user design is explicitely as is to reduce complexity --- docs/plan/drive.md | 54 +++++++++++++++++---- tests/api/search_basic.hurl | 78 ++++++++++++++++++++++++++++++- tests/fixtures/content-canary.txt | 1 + 3 files changed, 124 insertions(+), 9 deletions(-) create mode 100644 tests/fixtures/content-canary.txt diff --git a/docs/plan/drive.md b/docs/plan/drive.md index 5096d782..4947f6f4 100644 --- a/docs/plan/drive.md +++ b/docs/plan/drive.md @@ -882,14 +882,52 @@ field. change. 3. **Handler-side ReBAC re-verification** (defense in depth): after Tantivy returns hits, the `/api/search` handler - re-checks each `file_id` with the engine. Catches two cases - the drive_id filter can't: - - **Index staleness** — file just moved to a drive the - caller can't access; indexer hasn't caught up. - - **Per-file grants** — ReBAC can grant access to a single - file inside a drive the caller doesn't otherwise have. The - filter is drive-only; the re-check restores per-file - resolution. + re-checks each `file_id` with the engine. The re-check is + *subtractive only* — it can drop a Tantivy hit, never add + one. It catches: + - **Index staleness** — file just moved to a drive the caller + can't access; the indexer hasn't caught up so Tantivy still + returns the doc under its old drive_id (a false positive + from the caller's perspective). The re-check denies and the + hit drops. + + #### Known D0 scope limit — per-resource grants invisible in search + + The drive-only Must clause makes the inverse case structurally + invisible: a file or folder shared *directly* with the caller + via ReBAC, living in a drive the caller has no membership in, + never reaches the re-check because Tantivy already filtered the + doc out at index-query time. Concretely: + + - Alice grants Bob a per-file Read on `report.pdf` inside her + Personal drive. Bob has no drive grant on Alice's Personal. + - Bob's `accessible_drives` set doesn't include Alice's drive. + - Tantivy's `Must drive_id ∈ accessible_drives` rejects every + doc with Alice's drive_id, including `report.pdf`. + - Bob's search misses `report.pdf` even though `authz.check(Bob, + Read, File(report))` would say yes. + + This is **not a security issue** — Bob still can't access + anything he isn't entitled to. The trade-off is *discovery + only*: search doesn't surface directly-shared resources. The + "Shared with me" UI is the canonical surface for that workflow + (resources someone explicitly shared with you live in the + notification / inbox / share-listing flow, not in global search). + + Closing this gap is deferred. Two future moves, in order of cost: + - **Per-file grants** (low cost): one extra `role_grants` query + for `resource_type='file' AND subject ∈ caller_set`, widen + the Must clause to `drive_id ∈ A OR file_id ∈ F`. ~1 day of + work; deliverable when the "Shared with me" search surface is + prioritised. + - **Per-folder grants with cascade** (high cost): the ltree + subtree expansion is what makes it gnarly — a grant on folder + `F` should surface every descendant in search. Two viable + shapes: (a) reindex with a multi-valued `ancestor_folder_ids` + STRING field on each doc (schema v3, full reindex), or (b) + expand each granted folder to its subtree at query time + (per-grant recursive SQL on the hot path). Probably a dedicated + PR alongside the D1 URL-routing work. 4. **Token subjects cannot search**. `/api/search` returns 401 for token-authenticated callers (anonymous link tokens have access to one resource, not a drive — there is no meaningful diff --git a/tests/api/search_basic.hurl b/tests/api/search_basic.hurl index 4bc306a5..919ca997 100644 --- a/tests/api/search_basic.hurl +++ b/tests/api/search_basic.hurl @@ -153,7 +153,83 @@ body not contains "{{needle_file_id}}" # ───────────────────────────────────────────────────────────── -# 6 — Teardown: removing the folder recursively takes the file +# 6 — CONTENT-search cross-drive isolation (docs/plan/drive.md §11). +# The cross-user check above (step 5) verifies the NAME-search +# path. The Tantivy content index is a separate code path with +# its own filter: `Must drive_id ∈ accessible_drives`. This +# block pins it. +# +# Sequence: +# 6a. Admin uploads `content-canary.txt` whose body contains +# the distinctive phrase `ContentIndexCanaryXyzzy2026Drive`. +# 6b. Wait ~2s for the async content-index worker +# (`OXICLOUD_CONTENT_SEARCH_FLUSH_INTERVAL_MS` defaults +# to 1500ms) to drain the dirty queue and apply the +# Tantivy mutation. +# 6c. Admin searches for the phrase → MUST hit the file +# (the index works). +# 6d. Bob searches for the same phrase → MUST be empty, +# AND the response shape MUST carry no hidden-count +# leak (no `total`/`hidden`/etc. field that could +# reveal "you have N matches you can't see"). The +# pivot from `Must user_id = caller` to `Must drive_id +# ∈ accessible_drives` is the §11 security primitive; +# a regression here would be a cross-drive leak. +# ───────────────────────────────────────────────────────────── +POST {{base_url}}/api/files/upload +Authorization: Bearer {{admin_token}} +[MultipartFormData] +folder_id: {{search_folder_id}} +file: file,fixtures/content-canary.txt; text/plain + +HTTP 201 +[Captures] +canary_file_id: jsonpath "$.id" + + +# Drain the content-index worker. 2s exceeds the 1500ms flush +# interval comfortably; raise if a slower CI machine flakes. +GET {{base_url}}/api/search?query=ContentIndexCanaryXyzzy2026Drive +Authorization: Bearer {{admin_token}} +[Options] +delay: 2500ms + +HTTP 200 +[Asserts] +# Admin sees the content match — proves indexing landed. +jsonpath "$.files" count >= 1 +body contains "{{canary_file_id}}" + + +GET {{base_url}}/api/search?query=ContentIndexCanaryXyzzy2026Drive +Authorization: Bearer {{bob_token}} + +HTTP 200 +[Asserts] +# Bob has no access to admin's drive → Tantivy's Must-clause +# filters every doc that doesn't carry one of Bob's drive_ids, +# so the file vanishes entirely. +jsonpath "$.files" count == 0 +jsonpath "$.folders" count == 0 +body not contains "{{canary_file_id}}" +body not contains "ContentIndexCanaryXyzzy2026Drive" +# Anti-enum: every count the response surfaces must reflect the +# FILTERED set — i.e. zero when the caller has no accessible +# hits. The §11 rule is "no 'you have N hidden matches' field +# anywhere". `total_count` is a legitimate pagination count and +# is OK as long as it equals the filtered total (zero here). The +# other field names below MUST stay absent: a future field +# called `hidden_count`/`filtered`/etc. that reveals matches +# Bob can't see would be the regression. +jsonpath "$.total_count" == 0 +jsonpath "$.has_more" == false +jsonpath "$.hidden_count" not exists +jsonpath "$.filtered" not exists +jsonpath "$.total" not exists + + +# ───────────────────────────────────────────────────────────── +# 7 — Teardown: removing the folder recursively takes the files # with it, so a single DELETE is enough. # ───────────────────────────────────────────────────────────── DELETE {{base_url}}/api/folders/{{search_folder_id}} diff --git a/tests/fixtures/content-canary.txt b/tests/fixtures/content-canary.txt new file mode 100644 index 00000000..af9821a1 --- /dev/null +++ b/tests/fixtures/content-canary.txt @@ -0,0 +1 @@ +ContentIndexCanaryXyzzy2026Drive — this phrase is intentionally distinctive so that searches in the Tantivy content index can locate it unambiguously across test runs. Used by tests/api/search_basic.hurl to verify cross-drive content-search isolation (docs/plan/drive.md §11). Do not include this phrase in any other fixture.