mirror of
https://github.com/penpot/penpot.git
synced 2026-11-02 08:00:36 +00:00
* 🚑 Reject forged upload paths outside multipart Transit bodies can smuggle upload maps whose path points at any server file; the schema only checks the shape and handlers open the path directly. app.rpc now injects a server-side multipart flag that bodies cannot forge, and methods declare their upload keys with the new multipart-only-params metadata, enforced centrally after schema validation with a params-validation error. Covers all nine upload methods, including chunked and management flows. AI-assisted-by: muse-spark-1.3-contributor * 🐛 Sanitize raw explain in generic validation error bodies The generic :validation branch returned ex-data untouched, so errors carrying the raw malli explain (live schema objects, e.g. storage invalid-storage-metadata) broke JSON encoding mid-stream and clients got truncated bodies. Apply the same clean treatment the known codes already use: drop the raw explain and attach its humanized text. AI-assisted-by: muse-spark-1.3-contributor * 🐛 Add e2e path traversal tests for binfile and media upload Add two e2e suites that hand-write transit bodies with a forged ["~#path", ...] upload path (no transit library needed) and expect 400 validation/params-validation. They cover import-binfile :file, upload-file-media-object :content and upload-chunk :content, plus a plain-JSON string-path control case. Before the multipart-path fix they failed (500 io-exception, invalid-image, 200 stored chunk); after it they pass. AI-assisted-by: muse-spark-1.3-contributor * 🐛 Use multipart helper in binfile SVG test setup The SVG import setup uploaded through plain command!, so the new multipart-only guard rejected it with params-validation. Switch the setup to multipart-command! like the other upload tests. AI-assisted-by: muse-spark-1.3-contributor * 🐛 Use multipart helper in remaining upload tests The multipart-only upload guard rejects plain RPC calls carrying upload maps. Five tests with real files still used command! and failed with params-validation. Switch them to multipart-command! while keeping the forged-content tests on plain command!. AI-assisted-by: muse-spark-1.3-contributor
158 lines
6.7 KiB
Clojure
158 lines
6.7 KiB
Clojure
;; This Source Code Form is subject to the terms of the Mozilla Public
|
|
;; License, v. 2.0. If a copy of the MPL was not distributed with this
|
|
;; file, You can obtain one at http://mozilla.org/MPL/2.0/.
|
|
;;
|
|
;; Copyright (c) KALEIDOS SUBSIDIARY SL
|
|
|
|
(ns backend-tests.rpc-binfile-test
|
|
(:require
|
|
[app.binfile.v3 :as bf.v3]
|
|
[app.common.schema :as sm]
|
|
[app.common.uuid :as uuid]
|
|
[app.config :as cf]
|
|
[app.db :as db]
|
|
[app.rpc :as-alias rpc]
|
|
[app.rpc.commands.binfile :as binfile]
|
|
[backend-tests.helpers :as th]
|
|
[clojure.test :as t]
|
|
[datoteka.fs :as fs]
|
|
[mockery.core :refer [with-mocks]]))
|
|
|
|
(t/use-fixtures :once th/state-init)
|
|
(t/use-fixtures :each th/database-reset)
|
|
|
|
(t/deftest import-binfile-schema-omits-file-id
|
|
;; N1-06: file-id parameter must be removed from schema for security
|
|
(let [schema @#'binfile/schema:import-binfile
|
|
validator (sm/lazy-validator schema)
|
|
|
|
valid-params
|
|
{:name "test"
|
|
:project-id (uuid/random)
|
|
:version 3
|
|
:upload-id (uuid/random)}
|
|
|
|
params-with-file-id
|
|
(assoc valid-params :file-id (uuid/random))]
|
|
|
|
(t/is (true? (validator valid-params))
|
|
"params without file-id should be valid")
|
|
|
|
(t/is (not (contains? (sm/keys (second schema)) :file-id))
|
|
"file-id should not be a declared parameter")
|
|
|
|
;; Params with file-id should fail (schema closed)
|
|
(t/is (false? (validator params-with-file-id))
|
|
"params with file-id should be rejected")))
|
|
|
|
(t/deftest import-binfile-schema-accepts-path-object
|
|
;; Documents the schema hole: a server-side Path object in :file passes
|
|
;; validation on shape alone. The multipart guard in the handler (not the
|
|
;; schema) is what rejects client-forged paths.
|
|
(let [schema @#'binfile/schema:import-binfile
|
|
validator (sm/lazy-validator schema)]
|
|
(t/is (true? (validator {:name "pwned"
|
|
:project-id (uuid/random)
|
|
:version 3
|
|
:file {:filename "evil.zip"
|
|
:size 123
|
|
:path (fs/path "/etc/passwd")}})))))
|
|
|
|
(t/deftest import-binfile-rejects-forged-file-without-multipart
|
|
;; Path-traversal PoC at command level: a transit body like
|
|
;; {"~:file": {"~:path": ["~#path", "/etc/passwd"]}} decodes to this exact
|
|
;; map. Without a multipart request it must fail at params validation,
|
|
;; before any file is read.
|
|
(let [prof (th/create-profile* 1)
|
|
out (th/command! {::th/type :import-binfile
|
|
::rpc/profile-id (:id prof)
|
|
:name "pwned"
|
|
:version 3
|
|
:project-id (:default-project-id prof)
|
|
:file {:filename "evil.zip"
|
|
:size 123
|
|
:path (fs/path "/etc/passwd")}})]
|
|
(t/is (th/ex-of-code? (:error out) :params-validation))))
|
|
|
|
(t/deftest import-binfile-rejects-forged-file-with-forged-flag
|
|
;; The multipart flag is server-side: smuggling it in the body must not
|
|
;; bypass the guard (prepare-rpc-params overwrites it from the request).
|
|
(let [prof (th/create-profile* 1)
|
|
out (th/command! {::th/type :import-binfile
|
|
::rpc/profile-id (:id prof)
|
|
:app.rpc/is-multipart true
|
|
:name "pwned"
|
|
:version 3
|
|
:project-id (:default-project-id prof)
|
|
:file {:filename "evil.zip"
|
|
:size 123
|
|
:path (fs/path "/etc/passwd")}})]
|
|
(t/is (th/ex-of-code? (:error out) :params-validation))))
|
|
|
|
(t/deftest import-binfile-schema-rejects-unsupported-version
|
|
;; T1-N2-03: version parameter should be restricted to supported values (1 or 3)
|
|
(let [schema @#'binfile/schema:import-binfile
|
|
validator (sm/lazy-validator schema)
|
|
base-params {:name "test"
|
|
:project-id (uuid/random)
|
|
:upload-id (uuid/random)}]
|
|
|
|
;; Version 1 should be accepted
|
|
(t/is (true? (validator (assoc base-params :version 1)))
|
|
"version 1 should be valid")
|
|
|
|
;; Version 3 should be accepted
|
|
(t/is (true? (validator (assoc base-params :version 3)))
|
|
"version 3 should be valid")
|
|
|
|
;; Version 2 should be rejected
|
|
(t/is (false? (validator (assoc base-params :version 2)))
|
|
"version 2 should be rejected")
|
|
|
|
;; Version 0 should be rejected
|
|
(t/is (false? (validator (assoc base-params :version 0)))
|
|
"version 0 should be rejected")
|
|
|
|
;; Negative version should be rejected
|
|
(t/is (false? (validator (assoc base-params :version -1)))
|
|
"negative version should be rejected")
|
|
|
|
;; Version 4 should be rejected
|
|
(t/is (false? (validator (assoc base-params :version 4)))
|
|
"version 4 should be rejected")))
|
|
|
|
(t/deftest import-binfile-audit-events-carry-entity-ids
|
|
;; The imported files are created inside the importer, so the command emits a
|
|
;; create-file event per file for a local projection.
|
|
(with-mocks [audit-mock {:target 'app.loggers.audit/submit :return nil}]
|
|
(with-redefs [cf/flags (conj cf/flags :audit-log)]
|
|
(let [profile (th/create-profile* 1 {:is-active true})
|
|
team (th/create-team* 51 {:profile-id (:id profile)})
|
|
project (th/create-project* 51 {:profile-id (:id profile)
|
|
:team-id (:id team)})
|
|
;; A real row: `submit-file-created-event` reads the light
|
|
;; columns back, so a phantom id would emit no event.
|
|
file (th/create-file* 99 {:profile-id (:id profile)
|
|
:project-id (:id project)})
|
|
file-id (:id file)]
|
|
(with-redefs [bf.v3/import-files! (fn [_cfg] {:file-ids [file-id]})]
|
|
(#'binfile/import-binfile
|
|
th/*system*
|
|
{::rpc/profile-id (:id profile)
|
|
:profile-id (:id profile)
|
|
:project-id (:id project)
|
|
:version 3
|
|
:name "imported"
|
|
:file {:filename "package.penpot"
|
|
:path (th/tempfile "backend_tests/test_files/svg-attrs-camel-case.penpot")
|
|
:mtype "application/zip"
|
|
:size 1}}))
|
|
|
|
(let [events (->> (:call-args-list @audit-mock)
|
|
(map second)
|
|
(filter #(= "create-file" (:name %))))]
|
|
(t/is (= 1 (count events)))
|
|
(t/is (= file-id (get-in (first events) [:props :id])))
|
|
(t/is (= (:id project) (get-in (first events) [:props :project-id])))
|
|
(t/is (= (:id team) (get-in (first events) [:props :team-id]))))))))
|