From 52573be07472987703b2ce715c4c10919f4e153e Mon Sep 17 00:00:00 2001 From: Andrey Antukh Date: Tue, 25 Aug 2026 09:37:42 +0200 Subject: [PATCH] :bug: Enforce share-link page scope in get-page and get-file-fragment RPC commands (#11284) The get-page RPC command did not validate that the requested page-id was within the share-link's authorized :pages set, allowing share-link holders to read out-of-scope pages. The get-file-fragment command had the same issue. This fix adds page scope validation to get-page, rejecting requests for pages not in the share-link's :pages set with a :not-found error. For get-file-fragment, share-link access is denied entirely as fragments lack direct page-id mapping. The fix aligns these commands with the existing correct behavior in get-view-only-bundle, which already filters pages based on share-link scope. Closes #11281 AI-assisted-by: qwen3.7-plus --- backend/src/app/rpc/commands/audit.clj | 1 + backend/src/app/rpc/commands/files.clj | 15 ++- backend/test/backend_tests/rpc_file_test.clj | 100 +++++++++++++++++++ 3 files changed, 114 insertions(+), 2 deletions(-) diff --git a/backend/src/app/rpc/commands/audit.clj b/backend/src/app/rpc/commands/audit.clj index 0f4bf1c320..69777a1ddd 100644 --- a/backend/src/app/rpc/commands/audit.clj +++ b/backend/src/app/rpc/commands/audit.clj @@ -183,6 +183,7 @@ (sv/defmethod ::get-enabled-flags {::audit/skip true + ::rpc/auth false ::doc/skip true ::doc/added "1.20"} [_cfg _params] diff --git a/backend/src/app/rpc/commands/files.clj b/backend/src/app/rpc/commands/files.clj index 69bead539d..323ddf6c3f 100644 --- a/backend/src/app/rpc/commands/files.clj +++ b/backend/src/app/rpc/commands/files.clj @@ -245,6 +245,10 @@ [cfg {:keys [::rpc/profile-id file-id fragment-id share-id]}] (db/run! cfg (fn [cfg] (let [perms (perms/get-file-read-permissions cfg profile-id file-id share-id)] + (when (= :share-link (:type perms)) + (ex/raise :type :not-found + :code :object-not-found + :hint "object not found")) (check-read-permissions! perms) (-> (get-file-fragment cfg file-id fragment-id) (rph/with-http-cache long-cache-duration)))))) @@ -392,6 +396,14 @@ (let [perms (perms/get-file-read-permissions cfg profile-id file-id share-id) file (bfc/get-file cfg file-id :read-only? true) + resolved-page-id (or page-id (-> file :data :pages first)) + + _ (when (and (= :share-link (:type perms)) + (not (contains? (:pages perms) resolved-page-id))) + (ex/raise :type :not-found + :code :object-not-found + :hint "object not found")) + proj (db/get conn :project {:id (:project-id file)}) team (-> (db/get conn :team {:id (:team-id proj)}) @@ -402,8 +414,7 @@ (cfeat/check-file-features! (:features file))) page (binding [pmap/*load-fn* (partial feat.fdata/load-pointer cfg file-id)] - (let [page-id (or page-id (-> file :data :pages first)) - page (dm/get-in file [:data :pages-index page-id])] + (let [page (dm/get-in file [:data :pages-index resolved-page-id])] (if (pmap/pointer-map? page) (deref page) page)))] diff --git a/backend/test/backend_tests/rpc_file_test.clj b/backend/test/backend_tests/rpc_file_test.clj index 27e881dc45..da8fbbb1f7 100644 --- a/backend/test/backend_tests/rpc_file_test.clj +++ b/backend/test/backend_tests/rpc_file_test.clj @@ -2532,6 +2532,74 @@ share-links (:share-links (:result check))] (t/is (some #(= slink2-id (:id %)) share-links))))))) +(t/deftest share-link-page-scope-enforcement + (let [owner (th/create-profile* 1 {:is-active true}) + viewer (th/create-profile* 2 {:is-active true}) + proj-id (:default-project-id owner) + + file (th/create-file* 1 {:profile-id (:id owner) + :project-id proj-id + :is-shared false}) + + page-a (get-in file [:data :pages 0]) + page-b (uuid/random) + + ;; Add a second page to the file + _ (th/command! {::th/type :update-file + ::rpc/profile-id (:id owner) + :id (:id file) + :session-id (uuid/random) + :revn 0 + :vern 0 + :changes [{:type :add-page + :id page-b + :page {:id page-b + :name "Page B" + :options {} + :objects {}}}]}) + + ;; Create share-link scoped to page A only + share (th/command! {::th/type :create-share-link + ::rpc/profile-id (:id owner) + :file-id (:id file) + :pages #{page-a} + :who-comment "team" + :who-inspect "all"}) + share-id (get-in share [:result :id])] + + (t/testing "share-link holder can access authorized page" + (let [out (th/command! {::th/type :get-page + ::rpc/profile-id (:id viewer) + :file-id (:id file) + :page-id page-a + :share-id share-id})] + (t/is (nil? (:error out))) + (t/is (some? (:result out))))) + + (t/testing "share-link holder cannot access out-of-scope page" + (let [out (th/command! {::th/type :get-page + ::rpc/profile-id (:id viewer) + :file-id (:id file) + :page-id page-b + :share-id share-id}) + err (:error out) + edata (ex-data err)] + (t/is (th/ex-info? err)) + (t/is (= :not-found (:type edata))) + (t/is (= :object-not-found (:code edata))))) + + (t/testing "team member can access all pages" + (let [out-a (th/command! {::th/type :get-page + ::rpc/profile-id (:id owner) + :file-id (:id file) + :page-id page-a}) + out-b (th/command! {::th/type :get-page + ::rpc/profile-id (:id owner) + :file-id (:id file) + :page-id page-b})] + (t/is (nil? (:error out-a))) + (t/is (nil? (:error out-b))))))) + (t/deftest share-link-deletion-escape-hatches (let [owner (th/create-profile* 1 {:is-active true}) editor (th/create-profile* 2 {:is-active true}) @@ -2594,3 +2662,35 @@ ::rpc/profile-id (:id owner) :id slink-id})] (t/is (nil? (:error out))))))) + +(t/deftest share-link-fragment-access-denied + (let [owner (th/create-profile* 1 {:is-active true}) + viewer (th/create-profile* 2 {:is-active true}) + proj-id (:default-project-id owner) + + file (th/create-file* 1 {:profile-id (:id owner) + :project-id proj-id + :is-shared false}) + + page-a (get-in file [:data :pages 0]) + + ;; Create share-link + share (th/command! {::th/type :create-share-link + ::rpc/profile-id (:id owner) + :file-id (:id file) + :pages #{page-a} + :who-comment "team" + :who-inspect "all"}) + share-id (get-in share [:result :id])] + + (t/testing "share-link holder cannot access file fragments" + (let [out (th/command! {::th/type :get-file-fragment + ::rpc/profile-id (:id viewer) + :file-id (:id file) + :fragment-id (uuid/random) + :share-id share-id}) + err (:error out) + edata (ex-data err)] + (t/is (th/ex-info? err)) + (t/is (= :not-found (:type edata))) + (t/is (= :object-not-found (:code edata)))))))