Fetch only caller share-link in view-only bundle (#11657)

*  Fetch only caller share-link in view-only bundle

Share-link callers now resolve a single row with a composite
(id, file-id) predicate instead of loading all sibling rows
and filtering in memory. Membership path keeps full query.

Related #11633

AI-assisted-by: muse-spark-1.3-contributor

Signed-off-by: makesomethingshit <junsoo1172@gmail.com>

*  Add DB-access regression test for share-link bundle

The share-link path must resolve the caller row with a composite
(id, file-id) single-row lookup and never run the full
file-id query. Keep cross-file replay coverage.

Related #11633

AI-assisted-by: muse-spark-1.3-contributor
Signed-off-by: makesomethingshit <junsoo1172@gmail.com>

---------

Signed-off-by: makesomethingshit <junsoo1172@gmail.com>
Co-authored-by: Andrey Antukh <niwi@niwi.nz>
This commit is contained in:
makesomethingshit 2026-09-16 00:40:52 +09:00 committed by GitHub
parent ad7e035b63
commit 4a5c6fce7f
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
2 changed files with 70 additions and 13 deletions

View File

@ -89,18 +89,27 @@
(mapv (fn [{:keys [id] :as lib}]
(merge lib (bfc/get-file cfg id)))))
links (cond->> (->> (db/query conn :share-link {:file-id file-id})
(mapv (fn [row]
(-> row
(update :pages db/decode-pgarray #{})
;; NOTE: the flags are deprecated but are still present
;; on the table on old rows. The flags are pgarray and
;; for avoid decoding it (because they are no longer used
;; on frontend) we just dissoc the column attribute from
;; row.
(dissoc :flags)))))
(= :share-link (:type perms))
(filterv #(= (:id %) share-id)))
decode-link
(fn [row]
(-> row
(update :pages db/decode-pgarray #{})
;; NOTE: the flags are deprecated but are still present
;; on the table on old rows. The flags are pgarray and
;; for avoid decoding it (because they are no longer used
;; on frontend) we just dissoc the column attribute from
;; row.
(dissoc :flags)))
;; NOTE: on the share-link path we fetch at most the caller's own
;; row with a composite (id, file-id) predicate, so sibling tokens
;; never leave postgres. The membership path keeps the full list
;; the share-management UI needs. A nil share-id never falls back
;; to the full query; it simply resolves to an empty vector.
links (if (= :share-link (:type perms))
(if-some [row (db/get* conn :share-link {:id share-id :file-id file-id})]
[(decode-link row)]
[])
(mapv decode-link (db/query conn :share-link {:file-id file-id})))
fonts (db/query conn :team-font-variant
{:team-id (:id team)

View File

@ -204,4 +204,52 @@
;; Team member should see both share-links
(t/is (= 2 (count share-links)))
(t/is (some #(= link-a-id (:id %)) share-links))
(t/is (some #(= link-b-id (:id %)) share-links))))))
(t/is (some #(= link-b-id (:id %)) share-links))))
(t/testing "share-link viewer does not query sibling share-links"
;; Direct regression test for the predicate-pushdown invariant:
;; on the share-link path the bundle must resolve the caller's
;; row with a composite (id, file-id) single-row lookup and must
;; never run the full {:file-id} query that would load sibling
;; tokens into the backend process. The with-redefs spies only
;; record and delegate, following the instrumentation style used
;; elsewhere in this suite (e.g. auth-ldap-test).
(let [share-queries (atom [])
share-gets (atom [])
orig-query @#'db/query
orig-get* @#'db/get*]
(with-redefs [db/query (fn [conn table params & opts]
(when (= :share-link table)
(swap! share-queries conj params))
(apply orig-query conn table params opts))
db/get* (fn [conn table params & opts]
(when (= :share-link table)
(swap! share-gets conj params))
(apply orig-get* conn table params opts))]
(let [out (th/command! {::th/type :get-view-only-bundle
:share-id link-a-id
:file-id (:id file)})
result (:result out)]
(t/is (nil? (:error out)))
(t/is (= 1 (count (:share-links result))))
(t/is (= link-a-id (:id (first (:share-links result)))))
;; No full-file sibling query ran on this path.
(t/is (empty? @share-queries))
;; Only composite single-row lookups ran (the permission
;; check plus the bundle itself, same predicate in both).
(t/is (seq @share-gets))
(t/is (every? #(= {:id link-a-id :file-id (:id file)} %)
@share-gets))))))
(t/testing "cross-file share-id replay fails closed"
(let [other-file (th/create-file* 2 {:profile-id (:id owner)
:project-id proj-id
:is-shared false})
out (th/command! {::th/type :get-view-only-bundle
:share-id link-a-id
:file-id (:id other-file)})
error (:error out)
error-data (ex-data error)]
(t/is (th/ex-info? error))
(t/is (= :not-found (:type error-data)))
(t/is (= :object-not-found (:code error-data)))))))