From 3f8aee7933be80024b66e919a8e5ac5c451bc936 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?andr=C3=A9s=20gonz=C3=A1lez?= Date: Mon, 26 Oct 2026 12:38:02 +0100 Subject: [PATCH] :sparkles: Record imported file ids in the import-binfile audit event (#12491) The RPC layer emitted the event as soon as the handler returned the SSE response, before the import ran, so the event could not say which files the import created. Emit it from inside the SSE handler once the import ends, with the ids and count of the created files and whether the import completed or failed. Closes #12485 AI-assisted-by: claude-opus-5-5 --- backend/src/app/rpc/commands/binfile.clj | 47 +++++++++--- .../test/backend_tests/rpc_binfile_test.clj | 74 ++++++++++++++++++- 2 files changed, 109 insertions(+), 12 deletions(-) diff --git a/backend/src/app/rpc/commands/binfile.clj b/backend/src/app/rpc/commands/binfile.clj index fee8a98557..aef0befe42 100644 --- a/backend/src/app/rpc/commands/binfile.clj +++ b/backend/src/app/rpc/commands/binfile.clj @@ -17,7 +17,7 @@ [app.config :as cf] [app.db :as db] [app.http.sse :as sse] - [app.loggers.audit :as-alias audit] + [app.loggers.audit :as audit] [app.loggers.webhooks :as-alias webhooks] [app.media.validation :as media.v] [app.rpc :as-alias rpc] @@ -123,6 +123,34 @@ result)) +(defn- prepare-import-event + "Build the audit event of an import. The RPC layer cannot emit it + because the import runs later, inside the SSE response, and the + event must carry the ids of the files it creates." + [params manifest] + (-> (audit/event-from-rpc-params params) + (assoc :type "command") + (assoc :name "import-binfile") + (assoc :props (-> params + (dissoc :file) + (assoc :generated-by (:generated-by manifest)) + (assoc :referer (:referer manifest)) + (audit/clean-props))) + (assoc ::webhooks/event? true))) + +(defn- import-binfile-and-audit + [cfg params event] + (try + (let [file-ids (import-binfile cfg params)] + (audit/submit cfg (update event :props assoc + :file-ids (vec file-ids) + :file-count (count file-ids) + :status "completed")) + file-ids) + (catch Throwable cause + (audit/submit cfg (assoc-in event [:props :status] "failed")) + (throw cause)))) + (def ^:private schema:import-binfile [:and [:map {:title "import-binfile"} @@ -148,6 +176,7 @@ ["2.15" "Add upload-id param for chunked upload support"]] ::webhooks/event? true + ::audit/skip true ::sse/stream? true ::sm/params schema:import-binfile ::climit/id [[:import-binfile/by-profile ::rpc/profile-id] @@ -164,10 +193,6 @@ :binfile-v1 1 :binfile-v3 3)) - params (-> params - (assoc :profile-id profile-id) - (assoc :version version)) - manifest (case (int version) 1 nil @@ -175,10 +200,10 @@ (throw (ex-info (str "Unsupported binfile version: " version) {:type :validation :code :unsupported-version - :version version})))] + :version version}))) - (with-meta - (sse/response (partial import-binfile cfg params)) - {::audit/props {:file nil - :generated-by (:generated-by manifest) - :referer (:referer manifest)}}))) + params (assoc params :version version) + event (prepare-import-event params manifest) + params (assoc params :profile-id profile-id)] + + (sse/response (partial import-binfile-and-audit cfg params event)))) diff --git a/backend/test/backend_tests/rpc_binfile_test.clj b/backend/test/backend_tests/rpc_binfile_test.clj index b1b99762b0..09b8412e8b 100644 --- a/backend/test/backend_tests/rpc_binfile_test.clj +++ b/backend/test/backend_tests/rpc_binfile_test.clj @@ -6,12 +6,18 @@ (ns backend-tests.rpc-binfile-test (:require + [app.binfile.common :as bfc] + [app.binfile.v3 :as v3] [app.common.schema :as sm] [app.common.uuid :as uuid] [app.rpc :as-alias rpc] [app.rpc.commands.binfile :as binfile] + [app.storage.tmp :as tmp] [backend-tests.helpers :as th] - [clojure.test :as t])) + [clojure.test :as t] + [datoteka.fs :as fs] + [datoteka.io :as io] + [mockery.core :refer [with-mocks]])) (t/use-fixtures :once th/state-init) (t/use-fixtures :each th/database-reset) @@ -63,3 +69,69 @@ ;; Version 4 should be rejected (t/is (false? (validator (assoc base-params :version 4))) "version 4 should be rejected"))) + +(defn- export-file + [profile] + (let [file (th/create-file* 1 {:profile-id (:id profile) + :project-id (:default-project-id profile) + :is-shared false}) + output (tmp/tempfile :suffix ".zip")] + (v3/export-files! + (-> th/*system* + (assoc ::bfc/ids #{(:id file)}) + (assoc ::bfc/embed-assets false) + (assoc ::bfc/include-libraries false)) + (io/output-stream output)) + output)) + +(defn- import-file + [profile path] + (let [out (th/command! {::th/type :import-binfile + ::rpc/profile-id (:id profile) + :project-id (:default-project-id profile) + :name "imported" + :file {:filename "imported.penpot" + :size (fs/size path) + :path path + :mtype "application/zip"}})] + (t/is (nil? (:error out))) + (th/consume-sse (:result out)))) + +(defn- import-events + [mock] + (->> (:call-args-list @mock) + (map second) + (filter #(= "import-binfile" (:name %))))) + +(t/deftest import-binfile-audit-event-records-file-ids + (with-mocks [mock {:target 'app.loggers.audit/submit :return nil}] + (let [profile (th/create-profile* 1 {:is-active true}) + events (import-file profile (export-file profile)) + file-ids (-> events last second) + [event :as audit-events] (import-events mock)] + + (t/is (= :end (-> events last first))) + (t/is (= 1 (count audit-events))) + (t/is (= 1 (count file-ids))) + (t/is (= (vec file-ids) (-> event :props :file-ids))) + (t/is (= 1 (-> event :props :file-count))) + (t/is (= "completed" (-> event :props :status))) + (t/is (= (:default-project-id profile) (-> event :props :project-id))) + (t/is (= "penpot" (-> event :props :referer))) + (t/is (= (:id profile) (:profile-id event))) + (t/is (= "command" (:type event))) + (t/is (not (contains? (:props event) :file)))))) + +(t/deftest import-binfile-audit-event-records-failure + (with-mocks [mock {:target 'app.loggers.audit/submit :return nil}] + (let [profile (th/create-profile* 1 {:is-active true}) + path (export-file profile) + events (with-redefs [v3/import-files! + (fn [_] (throw (ex-info "boom" {:type :internal})))] + (import-file profile path)) + [event :as audit-events] (import-events mock)] + + (t/is (= :error (-> events last first))) + (t/is (= 1 (count audit-events))) + (t/is (= "failed" (-> event :props :status))) + (t/is (not (contains? (:props event) :file-ids))))))