Remove unused data from embed and public endpoints (#80510)

* Remove unused data from embed and public endpoints

Public and static-embed card/dashboard/document endpoints now return a
minimal placeholder dataset_query (a blank native query against the same
database) instead of selecting individual query keys. The frontend only
needs the database id here. Parameter combining and :param_fields
hydration in public-card now run before the card is trimmed since both
read template tags from the full query.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* Add tests for embed card and dashboard dataset_query

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* Assert strictly on dataset_query in public and embed tests

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* Handle native queries in Table viz

* Fix column shadowing in HeaderCellWithColumnInfo

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* Use param_fields for public question parameters

Public and static-embed card payloads carry param_fields instead of the
real query, so PublicOrEmbeddedQuestion now builds UI parameters from
them via getCardUiParametersFromParamFields, mirroring the saved-mode
dashboard path. The query-based helpers are renamed to the FromQuery /
FromParamFields scheme on both the card and dashboard sides, and the
ParameterWithTarget casts are dropped now that getParameterTargetField
accepts an optional target.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* Use param_fields for embed setup card parameters

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* Use param_fields for guest embed SDK question parameters

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* Route card UI parameters by param_fields presence

Restores getCardUiParameters as the entry point: it delegates to
getCardUiParametersFromParamFields when the card carries param_fields
(public and static-embed payloads) and to the query-based variant
otherwise. The SDK SQL parameter surfaces use it since they serve both
guest and authenticated contexts. Also reverts the dashboard helper
renames back to getSaved/getUnsavedDashboardUiParameters.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* Fix preview embed card param values

The preview-embed card values and remapping endpoints get their card via
card-for-unsigned-token -> public-card, which now replaces the query with
a blank native placeholder, so field-filter parameters could no longer be
resolved and the response schema rejected the nil result. Bind
qp.perms/*param-values-query* around the card fetch, the escape hatch
remove-card-non-public-columns already honors, matching the public
sharing values endpoints.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* Add preview embed test for remapped parameter fields

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* fix

* vscode fix

* Backfill template tag parameters in param_fields UI parameters

GET /api/card/:id returns parameters as stored, unlike the public and
embed endpoints which combine them with template tags, so
getCardUiParametersFromParamFields now derives parameters via
getParametersFromCard like the query-based variant always did. Guest
embed payloads are unaffected: their parameters arrive already combined
and the blank query derives nothing.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* Use getCardUiParameters dispatcher in SDK load-question

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* Guard applyTemplateTagParameters by param_fields presence

Cards whose payloads carry param_fields have parameters already combined
with template tags by the backend, so re-applying from the query would
wipe them. Keying the guard on the card payload keeps it in sync with
how the card was loaded, replacing the isGuestEmbed flag checks.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* Use getCardUiParameters everywhere and build test queries via Lib

Question.parameters and the query builder call sites go back to the
getCardUiParameters entry point, which now routes by param_fields
presence, leaving those files unchanged relative to master. The
template tag fallback test builds its query with
Lib.createTestJsNativeQuery instead of an inline legacy map.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* Make getCardUiParameters the only entry point

The FromQuery and FromParamFields variants are private to cards.ts now;
all consumers call getCardUiParameters, which routes by param_fields
presence.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* Build preview embed remapped-fields test query via lib

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* Restore null fallback in useAvailableParameters

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* Fix

* Preserve query type in stripped public and embed card queries

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* Add placeholder aggregations to stripped MBQL queries

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* Keep parameter template tags in stripped native queries and revert FE workarounds

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* Export parameter-template-tag? via import-vars

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* Add test for :param_fields hydration on GET /api/card/:id

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

---------

Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
Alexander Polyankin
2026-08-24 12:19:27 -04:00
committed by GitHub
parent c0ed9ab9af
commit 2350e6a952
9 changed files with 227 additions and 16 deletions

12
.vscode/settings.json vendored
View File

@@ -5,5 +5,15 @@
"gulp.autoDetect": "off",
"npm.autoDetect": "off",
"typescript.tsdk": "node_modules/typescript/lib",
"cssVariables.lookupFiles": ["**/*.d.css", "node_modules/@mantine/core/*.css"]
"typescript.tsserver.maxTsServerMemory": 8192,
"cssVariables.lookupFiles": ["**/*.d.css", "node_modules/@mantine/core/*.css"],
"files.watcherExclude": {
"**/.claude/worktrees/**": true,
"**/node_modules/**": true,
"**/.cpcache/**": true,
"**/target/**": true
},
"search.exclude": {
"**/.claude/worktrees": true
}
}

View File

@@ -15,6 +15,7 @@
[metabase.embedding.jwt :as embed]
[metabase.embedding.validation :as embedding.validation]
[metabase.parameters.schema :as parameters.schema]
[metabase.query-processor.middleware.permissions :as qp.perms]
[metabase.query-processor.pivot :as qp.pivot]
[metabase.request.core :as request]
[metabase.tiles.api :as api.tiles]
@@ -69,9 +70,12 @@
[:token string?]
[:param-key string?]]]
(let [unsigned-token (check-and-unsign token)
card (api.embed.common/card-for-unsigned-token
unsigned-token
:embedding-params (embed/get-in-unsigned-token-or-throw unsigned-token [:_embedding_params]))]
;; resolving param values needs the Card's real query, so skip the query stripping
;; in [[metabase.public-sharing-rest.api/remove-card-non-public-columns]]
card (binding [qp.perms/*param-values-query* true]
(api.embed.common/card-for-unsigned-token
unsigned-token
:embedding-params (embed/get-in-unsigned-token-or-throw unsigned-token [:_embedding_params])))]
(api.embed.common/card-param-values {:unsigned-token unsigned-token
:card card
:param-key param-key})))
@@ -87,9 +91,12 @@
[:param-key ms/NonBlankString]]
{:keys [value]} :- [:map [:value :string]]]
(let [unsigned-token (check-and-unsign token)
card (api.embed.common/card-for-unsigned-token
unsigned-token
:embedding-params (embed/get-in-unsigned-token-or-throw unsigned-token [:_embedding_params]))]
;; resolving param values needs the Card's real query, so skip the query stripping
;; in [[metabase.public-sharing-rest.api/remove-card-non-public-columns]]
card (binding [qp.perms/*param-values-query* true]
(api.embed.common/card-for-unsigned-token
unsigned-token
:embedding-params (embed/get-in-unsigned-token-or-throw unsigned-token [:_embedding_params])))]
(api.embed.common/card-param-remapped-value {:unsigned-token unsigned-token
:card card
:param-key param-key

View File

@@ -1,6 +1,7 @@
(ns metabase.public-sharing-rest.api
"Metabase API endpoints for viewing publicly-accessible Cards and Dashboards."
(:require
[clojure.string :as str]
[hiccup.core :as hiccup]
[medley.core :as m]
[metabase.actions.core :as actions]
@@ -12,6 +13,8 @@
[metabase.dashboards.schema :as dashboards.schema]
[metabase.documents.prose-mirror :as prose-mirror]
[metabase.events.core :as events]
[metabase.lib.core :as lib]
[metabase.lib.metadata :as lib.metadata]
[metabase.lib.schema.info :as lib.schema.info]
[metabase.models.interface :as mi]
[metabase.parameters.dashboard :as parameters.dashboard]
@@ -61,9 +64,31 @@
[card]
(assoc card :parameters (qp.card/combined-parameters-and-template-tags card)))
(defn- blank-dataset-query
"Replace the contents of `query` with a blank query of the same type, so the query contents themselves (SQL,
filters, aggregations, etc.) are never exposed to the general public. Native queries keep their parameter template
tags so the frontend can still derive parameters from them. MBQL queries keep only their source table or source
card, plus a placeholder `:count` aggregation per original aggregation, so the frontend can still tell MBQL and
native queries apart (e.g. to use the pivot endpoints) and resolve `:aggregation` column refs."
[query]
(if (lib/native? query)
(let [text (->> (lib/template-tags query)
(filter queries/parameter-template-tag?)
(map #(str "{{" (:name %) "}}"))
(str/join " "))]
(lib/with-native-query query (if (str/blank? text) "-" text)))
(if-let [source (or (some->> (lib/primary-source-table-id query) (lib.metadata/table query))
(some->> (lib/primary-source-card-id query) (lib.metadata/card query)))]
(reduce lib/aggregate
(lib/query query source)
(repeatedly (count (lib/aggregations query -1)) lib/count))
(lib/native-query query "-"))))
(defn remove-card-non-public-columns
"Remove everything from public `card` that shouldn't be visible to the general public.
The `:dataset_query` is replaced with a blank query of the same type via [[blank-dataset-query]].
This function is used by both OSS (for public cards) and EE (for cards in public documents) to ensure
consistent filtering of sensitive fields across all public sharing endpoints."
[card]
@@ -73,8 +98,11 @@
(mi/instance
:model/Card
(-> card
(select-keys [:id :name :description :display :visualization_settings :parameters :entity_id :dataset_query])
(update :dataset_query select-keys [:lib/metadata :lib/type :database :stages])))))
(select-keys [:id :name :description :display :visualization_settings :parameters :param_fields :entity_id
:dataset_query])
(update :dataset_query (fn [query]
(cond-> query
(seq query) blank-dataset-query)))))))
(defn public-card
"Return a public Card matching key-value `conditions`, removing all columns that should not be visible to the general
@@ -84,9 +112,9 @@
(-> (api/check-404 (apply t2/select-one [:model/Card :id :dataset_query :description :display :name :parameters
:visualization_settings :card_schema]
:archived false, conditions))
remove-card-non-public-columns
combine-parameters-and-template-tags
(t2/hydrate :param_fields))))
(t2/hydrate :param_fields)
remove-card-non-public-columns)))
(defn- card-with-uuid [uuid] (public-card :public_uuid uuid))

View File

@@ -37,6 +37,7 @@
maybe-unverify!
model-supports-implicit-actions?
model?
parameter-template-tag?
sole-dashboard-id
starting-card-schema-version
update-card!

View File

@@ -339,6 +339,16 @@
;;; NOTE: this should mirror `getTemplateTagParameters` in frontend/src/metabase-lib/parameters/utils/template-tags.ts
;;; If this function moves you should update the comment that links to this one (#40013)
;;;
(mu/defn parameter-template-tag? :- :boolean
"Whether a parameter is created for this template tag, as opposed to tags that splice content into the query itself,
like snippets, card references, and tables."
[{tag-type :type, widget-type :widget-type} :- [:maybe ::lib.schema.template-tag/template-tag]]
(boolean
(and tag-type
(or (contains? lib.schema.template-tag/raw-value-template-tag-types tag-type)
(= tag-type :temporal-unit)
(and (= tag-type :dimension) widget-type (not= widget-type :none))))))
;;; TODO -- does this belong HERE or in the `parameters` module?
(mu/defn template-tag-parameters :- ::parameters.schema/parameters
"Transforms native query's `template-tags` into `parameters`.
@@ -346,10 +356,7 @@
should always be there. Apparently lots of e2e tests are sloppy about this so this is included as a convenience."
[card :- [:maybe ::queries.schema/card]]
(for [{tag-type :type, widget-type :widget-type, :as tag} (some-> card :dataset_query not-empty lib/all-template-tags)
:when (and tag-type
(or (contains? lib.schema.template-tag/raw-value-template-tag-types tag-type)
(= tag-type :temporal-unit)
(and (= tag-type :dimension) widget-type (not= widget-type :none))))]
:when (parameter-template-tag? tag)]
{:id (:id tag)
:type (or widget-type (case tag-type
:temporal-unit :temporal-unit

View File

@@ -151,7 +151,9 @@
:display "table"
:visualization_settings {}
:dataset_query {:lib/type "mbql/query"
:stages [{:lib/type "mbql.stage/mbql"}]}
:stages [{:lib/type "mbql.stage/mbql"
:source-table int?
:aggregation [["count" {:lib/uuid string?}]]}]}
:parameters []
:param_fields {}})
@@ -171,6 +173,19 @@
(is (=? successful-card-info
(client/client :get 200 (card-url card)))))))
(deftest embed-card-strips-dataset-query-test
(testing "GET /api/embed/card/:token replaces the Card's query with a blank native query"
(with-embedding-enabled-and-new-secret-key!
(with-temp-card [card {:enable_embedding true
:dataset_query (mt/native-query {:query "SELECT id FROM venues"})}]
(let [{:keys [dataset_query]} (client/client :get 200 (card-url card))]
(is (= {:lib/type "mbql/query"
:database (mt/id)
:stages [{:lib/type "mbql.stage/native"
:native "-"
:template-tags []}]}
dataset_query)))))))
(deftest we-should-fail-when-attempting-to-use-an-expired-token
(with-embedding-enabled-and-new-secret-key!
(with-temp-card [card {:enable_embedding true}]
@@ -572,6 +587,18 @@
(is (=? successful-dashboard-info
(client/client :get 200 (dashboard-url (:entity_id dash) dash)))))))
(deftest embed-dashboard-strips-dataset-query-test
(testing "GET /api/embed/dashboard/:token replaces each Card's query with a blank query"
(with-embedding-enabled-and-new-secret-key!
(with-temp-dashcard [dashcard {:dash {:enable_embedding true}}]
(let [response (client/client :get 200 (dashboard-url (:dashboard_id dashcard)))]
(is (=? {:lib/type "mbql/query"
:database (mt/id)
:stages [{:lib/type "mbql.stage/mbql"
:source-table (mt/id :venues)
:aggregation [["count" {:lib/uuid string?}]]}]}
(-> response :dashcards first :card :dataset_query))))))))
(deftest bad-dashboard-id-fails
(with-embedding-enabled-and-new-secret-key!
(let [dashboard-url (str "embed/dashboard/" (sign {:resource {:dashboard "8"} :params {}}))]

View File

@@ -4,10 +4,14 @@
metabase.test.data/run-mbql-query {:namespaces [metabase.embedding-rest.api.preview-embed-test]}}}}}}
(:require
[buddy.sign.jwt :as jwt]
[clojure.set :as set]
[clojure.test :refer :all]
[metabase.dashboards-rest.api-test :as api.dashboard-test]
[metabase.embedding-rest.api.embed-test :as embed-test]
[metabase.embedding-rest.api.preview-embed :as api.preview-embed]
[metabase.lib.core :as lib]
[metabase.lib.metadata :as lib.metadata]
[metabase.queries-rest.api.card-test :as api.card-test]
[metabase.query-processor.pivot.test-util :as api.pivots]
[metabase.test :as mt]
[metabase.tiles.api-test :as tiles.api-test]
@@ -556,6 +560,69 @@
(format "preview_embed/card/%s/params/%s/values"
signed-token "_STATIC_CATEGORY_"))))))))))
(deftest card-params-values-field-filter-test
(testing "GET /api/preview_embed/card/:token/params/:param-key/values with a field filter parameter"
(embed-test/with-embedding-enabled-and-new-secret-key!
(api.card-test/with-card-param-values-fixtures [{:keys [field-filter-card param-keys]}]
(let [embedding-params (zipmap (map (comp keyword :slug) (:parameters field-filter-card))
(repeat "enabled"))
signed-token (embed-test/card-token field-filter-card
{:_embedding_params embedding-params})
response (mt/user-http-request :crowberto :get 200
(format "preview_embed/card/%s/params/%s/values"
signed-token (:field-values param-keys)))]
(is (false? (:has_more_values response)))
(is (set/subset? #{["20th Century Cafe"] ["33 Taps"]}
(-> response :values set))))))))
(deftest card-params-values-remapped-fields-test
(testing "preview embed card values/remapping endpoints work for parameters mapped to remapped fields"
(embed-test/with-embedding-enabled-and-new-secret-key!
(mt/with-column-remappings [orders.quantity {5 "N5"}
orders.product_id products.title]
(mt/with-temp [:model/Card card
(let [mp (mt/metadata-provider)]
{:dataset_query
(-> (lib/native-query mp "SELECT * FROM ORDERS JOIN PEOPLE ON ORDERS.USER_ID = PEOPLE.ID WHERE {{quantity}} AND {{product_id_fk}} AND {{user_id_pk}}")
(lib/with-template-tags
{"quantity" {:id "quantity"
:name "quantity"
:display-name "Internal"
:type :dimension
:widget-type :number/=
:dimension (lib/ref (lib.metadata/field mp (mt/id :orders :quantity)))}
"product_id_fk" {:id "product_id_fk"
:name "product_id_fk"
:display-name "FK"
:type :dimension
:widget-type :id
:dimension (lib/ref (lib.metadata/field mp (mt/id :orders :product_id)))}
"user_id_pk" {:id "user_id_pk"
:name "user_id_pk"
:display-name "PK->Name"
:type :dimension
:widget-type :id
:dimension (lib/ref (lib.metadata/field mp (mt/id :people :id)))}}))
:parameters [{:id "quantity", :name "Internal", :slug "quantity", :type "number/="
:target ["dimension" ["template-tag" "quantity"]]}
{:id "product_id_fk", :name "FK", :slug "product_id_fk", :type "id"
:target ["dimension" ["template-tag" "product_id_fk"]]}
{:id "user_id_pk", :name "PK->Name", :slug "user_id_pk", :type "id"
:target ["dimension" ["template-tag" "user_id_pk"]]}]})]
(let [token (embed-test/card-token card {:_embedding_params {:quantity "enabled"
:product_id_fk "enabled"
:user_id_pk "enabled"}})]
(testing "values for internally-remapped param"
(is (map? (mt/user-http-request :crowberto :get 200
(format "preview_embed/card/%s/params/quantity/values" token)))))
(testing "values for FK-remapped param"
(is (map? (mt/user-http-request :crowberto :get 200
(format "preview_embed/card/%s/params/product_id_fk/values" token)))))
(testing "remapping for PK param"
(is (some? (mt/user-http-request :crowberto :get 200
(format "preview_embed/card/%s/params/user_id_pk/remapping" token)
:value "1"))))))))))
(deftest params-with-static-list-test
(testing "embedding with parameter that has source is a static list"
(with-embedding-enabled-and-new-secret-key!

View File

@@ -144,6 +144,62 @@
(is (= "Not found."
(client/client :get 404 (str "public/card/" uuid))))))))))
(deftest fetch-card-strips-dataset-query-test
(testing "GET /api/public/card/:uuid replaces the Card's query with a blank query so its contents are not exposed"
(mt/with-temporary-setting-values [enable-public-sharing true]
(with-temp-public-card [{uuid :public_uuid}]
(let [{:keys [dataset_query]} (client/client :get 200 (str "public/card/" uuid))]
(is (=? {:lib/type "mbql/query"
:database (mt/id)
:stages [{:lib/type "mbql.stage/mbql"
:source-table (mt/id :venues)
:aggregation [["count" {:lib/uuid string?}]]}]}
dataset_query)))))))
(deftest fetch-card-strips-native-query-keeps-parameter-tags-test
(testing "GET /api/public/card/:uuid strips the native query text but keeps parameter template tags"
(mt/with-temporary-setting-values [enable-public-sharing true]
(mt/with-temp [:model/NativeQuerySnippet snippet {:name "greeting" :content "'hello'"}]
(with-temp-public-card
[{uuid :public_uuid}
(let [mp (mt/metadata-provider)]
{:dataset_query
(-> (lib/native-query mp "SELECT {{snippet: greeting}} FROM venues WHERE {{price}}")
(lib/with-template-tags
{"price" {:id "_PRICE_"
:name "price"
:display-name "Price"
:type :dimension
:dimension (lib/ref (lib.metadata/field mp (mt/id :venues :price)))
:widget-type :category}
"snippet: greeting" {:type :snippet
:name "snippet: greeting"
:id (str (random-uuid))
:snippet-name "greeting"
:display-name "Snippet: Greeting"
:snippet-id (:id snippet)}}))})]
(let [{:keys [dataset_query]} (client/client :get 200 (str "public/card/" uuid))]
(is (=? {:lib/type "mbql/query"
:database (mt/id)
:stages [{:lib/type "mbql.stage/native"
:native "{{price}}"
:template-tags [{:name "price"
:type "dimension"
:widget-type "category"}]}]}
dataset_query))))))))
(deftest fetch-dashboard-strips-dataset-query-test
(testing "GET /api/public/dashboard/:uuid replaces each Card's query with a blank query so its contents are not exposed"
(mt/with-temporary-setting-values [enable-public-sharing true]
(with-temp-public-dashboard-and-card [dash _card]
(let [response (client/client :get 200 (str "public/dashboard/" (:public_uuid dash)))]
(is (=? {:lib/type "mbql/query"
:database (mt/id)
:stages [{:lib/type "mbql.stage/mbql"
:source-table (mt/id :venues)
:aggregation [["count" {:lib/uuid string?}]]}]}
(-> response :dashcards first :card :dataset_query))))))))
(deftest public-queries-are-counted-test
(testing "GET /api/public/card/:uuid/query counts as a public query"
(mt/with-temporary-setting-values [enable-public-sharing true]

View File

@@ -3700,6 +3700,14 @@
(is (set/subset? #{["Barney's Beanery"] ["bigmista's barbecue"]}
(-> response :values set)))))))))
(deftest param-fields-hydrated-test
(testing "GET /api/card/:id hydrates :param_fields with the fields referenced by the card's template tags"
(with-card-param-values-fixtures [{:keys [field-filter-card]}]
(is (=? {:name_param_id [{:id (mt/id :venues :name)
:table_id (mt/id :venues)
:display_name "Name"}]}
(:param_fields (mt/user-http-request :rasta :get 200 (format "card/%d" (:id field-filter-card)))))))))
(deftest param-fields-excluded-without-view-data-permission-test
(testing "param_fields should not include fields for tables where the user lacks view-data permission"
(with-card-param-values-fixtures [{:keys [field-filter-card]}]