commit f788632842b0a8a0e8e8c319b27e1433054605f6 parent 63d615a5b0ed5041e7f2e810e9996268c6222db5 Author: Christian Grothoff <christian@grothoff.org> Date: Thu, 29 Oct 2026 19:54:14 +0100 enforce constraints in DB schema on migration Diffstat:
15 files changed, 333 insertions(+), 29 deletions(-)
diff --git a/src/backend/taler-merchant-httpd_patch-private-products-PRODUCT_ID.c b/src/backend/taler-merchant-httpd_patch-private-products-PRODUCT_ID.c @@ -151,6 +151,7 @@ TMH_private_patch_products_ID ( bool stock_reduced; bool no_group; bool no_pot; + bool no_unit; GNUNET_assert (NULL != mi); GNUNET_assert (NULL != product_id); @@ -376,7 +377,8 @@ TMH_private_patch_products_ID ( &sold_reduced, &stock_reduced, &no_group, - &no_pot); + &no_pot, + &no_unit); switch (qs) { case GNUNET_DB_STATUS_HARD_ERROR: @@ -404,6 +406,15 @@ TMH_private_patch_products_ID ( break; } + if (no_unit) + { + ret = TALER_MHD_reply_with_error ( + connection, + MHD_HTTP_NOT_FOUND, + TALER_EC_MERCHANT_GENERIC_UNIT_UNKNOWN, + pd.unit); + goto cleanup; + } if (-1 != no_cat) { char cat_str[24]; diff --git a/src/backend/taler-merchant-httpd_post-management-instances.c b/src/backend/taler-merchant-httpd_post-management-instances.c @@ -226,27 +226,19 @@ post_instances (const struct TMH_RequestHandler *rh, } } - /* check 'id' well-formed */ + /* check extra constraints on instance IDs beyond is 'slug' */ { - static bool once; - static regex_t reg; bool id_wellformed = true; - if (! once) - { - once = true; - GNUNET_assert (0 == - regcomp (®, - "^[A-Za-z0-9][A-Za-z0-9_.@-]+$", - REG_EXTENDED)); - } - - if (0 != regexec (®, - is.id, - 0, NULL, 0)) - id_wellformed = false; + if ('\0' == is.id[1]) + id_wellformed = false; /* at least two characters */ + if (! ( ( ('a' <= is.id[0]) && ('z' >= is.id[0]) ) || + ( ('A' <= is.id[0]) && ('Z' >= is.id[0]) ) || + ( ('0' <= is.id[0]) && ('9' >= is.id[0]) ) ) ) + id_wellformed = false; /* must start alphanumeric */ if (! id_wellformed) { + GNUNET_break_op (0); GNUNET_JSON_parse_free (spec); GNUNET_free (is.phone); return TALER_MHD_reply_with_error (connection, diff --git a/src/backend/taler-merchant-httpd_post-private-products.c b/src/backend/taler-merchant-httpd_post-private-products.c @@ -133,6 +133,7 @@ TMH_private_post_products (const struct TMH_RequestHandler *rh, ssize_t no_cat; bool no_group; bool no_pot; + bool no_unit; enum GNUNET_DB_QueryStatus qs; enum MHD_Result ret; @@ -353,7 +354,8 @@ TMH_private_post_products (const struct TMH_RequestHandler *rh, &conflict, &no_cat, &no_group, - &no_pot); + &no_pot, + &no_unit); switch (qs) { case GNUNET_DB_STATUS_HARD_ERROR: @@ -376,6 +378,15 @@ TMH_private_post_products (const struct TMH_RequestHandler *rh, case GNUNET_DB_STATUS_SUCCESS_ONE_RESULT: break; } + if (no_unit) + { + ret = TALER_MHD_reply_with_error ( + connection, + MHD_HTTP_NOT_FOUND, + TALER_EC_MERCHANT_GENERIC_UNIT_UNKNOWN, + pd.unit); + goto cleanup; + } if (no_group) { ret = TALER_MHD_reply_with_error ( diff --git a/src/backenddb/insert_product.c b/src/backenddb/insert_product.c @@ -36,7 +36,8 @@ TALER_MERCHANTDB_insert_product ( bool *conflict, ssize_t *no_cat, bool *no_group, - bool *no_pot) + bool *no_pot, + bool *no_unit) { struct GNUNET_PQ_QueryParam params[] = { GNUNET_PQ_query_param_string (product_id), @@ -83,6 +84,8 @@ TALER_MERCHANTDB_insert_product ( no_group), GNUNET_PQ_result_spec_bool ("no_pot", no_pot), + GNUNET_PQ_result_spec_bool ("no_unit", + no_unit), GNUNET_PQ_result_spec_end }; enum GNUNET_DB_QueryStatus qs; @@ -96,6 +99,7 @@ TALER_MERCHANTDB_insert_product ( ",out_no_cat AS no_cat" ",out_no_group AS no_group" ",out_no_pot AS no_pot" + ",out_no_unit AS no_unit" " FROM merchant_do_insert_product" "($1, $2, $3::TEXT::JSONB, $4, $5, $6::TEXT::JSONB, $7" ",$8, $9, $10, $11, $12::TEXT::JSONB, $13, $14, $15" diff --git a/src/backenddb/insert_product.sql b/src/backenddb/insert_product.sql @@ -38,7 +38,8 @@ CREATE FUNCTION merchant_do_insert_product ( OUT out_conflict BOOL, OUT out_no_cat INT8, OUT out_no_group BOOL, - OUT out_no_pot BOOL) + OUT out_no_pot BOOL, + OUT out_no_unit BOOL) LANGUAGE plpgsql AS $$ DECLARE @@ -49,6 +50,29 @@ BEGIN out_no_group = FALSE; out_no_pot = FALSE; +out_no_unit = FALSE; + +-- Check the unit exists. 'unit' is a slug, but nothing tied it to the +-- unit tables, so a product could name a unit that GET /private/units +-- does not know. Checked first, before anything is modified. +-- NOTE: a *repeated* POST of a product that was stored with an unknown +-- unit by an older backend now reports the unknown unit instead of +-- replying 204 idempotent. That is the point: the unit has to be +-- created before the product can be re-sent. +PERFORM FROM merchant_custom_units + WHERE unit=in_unit; +IF NOT FOUND +THEN + PERFORM FROM merchant.merchant_builtin_units + WHERE unit=in_unit; + IF NOT FOUND + THEN + out_no_unit=TRUE; + out_conflict=FALSE; + out_no_cat=NULL; + RETURN; + END IF; +END IF; IF in_product_group_id IS NOT NULL THEN diff --git a/src/backenddb/sql-schema/merchant-0051.sql b/src/backenddb/sql-schema/merchant-0051.sql @@ -0,0 +1,140 @@ +-- +-- This file is part of TALER +-- Copyright (C) 2026 Taler Systems SA +-- +-- TALER is free software; you can redistribute it and/or modify it under the +-- terms of the GNU General Public License as published by the Free Software +-- Foundation; either version 3, or (at your option) any later version. +-- +-- TALER is distributed in the hope that it will be useful, but WITHOUT ANY +-- WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS FOR +-- A PARTICULAR PURPOSE. See the GNU General Public License for more details. +-- +-- You should have received a copy of the GNU General Public License along with +-- TALER; see the file COPYING. If not, see <http://www.gnu.org/licenses/> + +-- @file merchant-0051.sql +-- @brief Constrain the identifier columns to the slug character set + +-- Every column constrained here holds what the protocol calls a slug, and +-- TALER_is_slug() has guarded the REST input that writes it only since +-- 2026-05-21 (identifiers other than the order ID), 2026-07-05 (instance ID) +-- and 2026-08-04 (order ID, product unit). An older backend accepted +-- anything a JSON string can hold, among it '@' in an instance ID, which its +-- regex allowed on purpose. The read-path parsers are strict now, so such a +-- row makes GET /private/orders, GET /management/instances and the payment +-- of the affected order fail, and nothing in the schema says which row is at +-- fault. +-- +-- The constraints are therefore added NOT VALID: new and updated rows have +-- to be well-formed, while existing rows are left alone so that the upgrade +-- cannot fail on data an operator cannot see yet. To find the offending +-- rows, validate a constraint by hand; it reports the first row it rejects: +-- +-- ALTER TABLE merchant_instance_1.merchant_orders +-- VALIDATE CONSTRAINT merchant_orders_order_id_slug_check; +-- +-- A constraint that validates cleanly is marked valid and stays so. +-- +-- The character set is the ASCII one of TALER_is_slug(): letters, digits, +-- '-', '_', '.', ':' and '~', at least one character, and neither '.' nor +-- '..' (both are path components). Note that isalnum() is locale-dependent +-- in C while the class below is not; the C side has to be pinned to ASCII +-- as well for the two to agree. +-- +-- Deliberately not covered: merchant_statistic_bucket_meta.slug and +-- merchant_statistic_interval_meta.slug. Those rows come from SQL an +-- operator supplies, not from REST input, and failing their INSERT is not +-- this migration's business. + +BEGIN; + +SELECT _v.register_patch('merchant-0051', NULL, NULL); + +SET search_path TO merchant; + +CREATE PROCEDURE merchant.merchant_0051_init(s TEXT) + LANGUAGE plpgsql + AS $$ +DECLARE + my_slug_re TEXT DEFAULT '^[A-Za-z0-9._:~-]+$'; + my_conname TEXT; + rec RECORD; +BEGIN + FOR rec IN + SELECT * + FROM (VALUES + ('merchant_inventory', 'product_id') + ,('merchant_inventory', 'unit') + ,('merchant_custom_units', 'unit') + ,('merchant_orders', 'order_id') + ,('merchant_contract_terms', 'order_id') + ,('merchant_template', 'template_id') + ,('merchant_webhook', 'webhook_id') + ,('merchant_otp_devices', 'otp_id') + ,('merchant_token_families', 'slug') + ) AS t(tbl, col) + LOOP + my_conname = format('%s_%s_slug_check', rec.tbl, rec.col); + + -- The procedure also runs from merchant.create_instance_schema() for + -- every new instance, so do not add a constraint twice. + PERFORM + FROM pg_constraint c + JOIN pg_namespace n + ON (n.oid = c.connamespace) + WHERE n.nspname = s + AND c.conname = my_conname; + IF FOUND + THEN + CONTINUE; + END IF; + + EXECUTE format('ALTER TABLE %I.%I' + ' ADD CONSTRAINT %I' + ' CHECK ( (%I ~ %L)' + ' AND (%I <> %L)' + ' AND (%I <> %L) )' + ' NOT VALID', + s, rec.tbl, + my_conname, + rec.col, my_slug_re, + rec.col, '.', + rec.col, '..'); + END LOOP; +END $$; + +COMMENT ON PROCEDURE merchant.merchant_0051_init(TEXT) + IS 'Constrains the slug columns of one instance schema to the slug character set, without validating the rows already there.'; + + +INSERT INTO merchant.instance_fixups + (migration_name + ,version) + VALUES + ('merchant_0051_init' + ,51); +-- Apply new fix-up to existing instances +CALL merchant.fixup_instance_schema (51::INT8); + +-- Also constrain instance IDs. +DO $$ +BEGIN + PERFORM + FROM pg_constraint c + JOIN pg_namespace n + ON (n.oid = c.connamespace) + WHERE n.nspname = 'merchant' + AND c.conname = 'merchant_instances_merchant_id_slug_check'; + IF NOT FOUND + THEN + ALTER TABLE merchant.merchant_instances + ADD CONSTRAINT merchant_instances_merchant_id_slug_check + CHECK ( (merchant_id ~ '^[A-Za-z0-9._:~-]+$') + AND (merchant_id <> '.') + AND (merchant_id <> '..') ) + NOT VALID; + END IF; +END $$; + +COMMIT; diff --git a/src/backenddb/sql-schema/meson.build b/src/backenddb/sql-schema/meson.build @@ -150,6 +150,7 @@ generated_sql = [ ['merchant-0048.sql'], ['merchant-0049.sql'], ['merchant-0050.sql'], + ['merchant-0051.sql'], ] migration_sql = [] diff --git a/src/backenddb/test_merchantdb.c b/src/backenddb/test_merchantdb.c @@ -1298,7 +1298,7 @@ make_product (const char *id, product->product.description = "This is a test product"; product->product.description_i18n = json_array (); GNUNET_assert (NULL != product->product.description_i18n); - product->product.unit = "boxes"; + product->product.unit = "Set"; product->product.minimum_age = 0; product->product.price_array = &htwenty40; product->product.price_array_length = 1; @@ -1400,6 +1400,7 @@ test_insert_product (const struct InstanceData *instance, ssize_t no_cat; bool no_group; bool no_pot; + bool no_unit; if (expect_no_instance) { @@ -1420,7 +1421,8 @@ test_insert_product (const struct InstanceData *instance, &conflict, &no_cat, &no_group, - &no_pot), + &no_pot, + &no_unit), "Insert product failed\n"); if (expected_result > 0) { @@ -1462,6 +1464,7 @@ test_update_product (const struct InstanceData *instance, bool stocked_reduced; bool no_group; bool no_pot; + bool no_unit; if (expect_no_instance) { @@ -1486,7 +1489,8 @@ test_update_product (const struct InstanceData *instance, &sold_reduced, &stocked_reduced, &no_group, - &no_pot), + &no_pot, + &no_unit), "Update product failed\n"); if (expected_result > 0) { @@ -1741,7 +1745,7 @@ pre_test_products (struct TestProducts_Closure *cls) make_product ("test_products_pd_1", &cls->products[1]); cls->products[1].product.description = "This is a another test product"; - cls->products[1].product.unit = "cans"; + cls->products[1].product.unit = "VolumeUnitLitre"; cls->products[1].product.minimum_age = 0; GNUNET_assert (GNUNET_OK == @@ -2242,7 +2246,7 @@ run_test_products (struct TestProducts_Closure *cls) cls->products[0].product.description_i18n, json_string ( "description in another language"))); - cls->products[0].product.unit = "barrels"; + cls->products[0].product.unit = "VolumeUnitM3"; { static struct TALER_Amount seven68; diff --git a/src/backenddb/update_product.c b/src/backenddb/update_product.c @@ -39,7 +39,8 @@ TALER_MERCHANTDB_update_product ( bool *sold_reduced, bool *stocked_reduced, bool *no_group, - bool *no_pot) + bool *no_pot, + bool *no_unit) { struct GNUNET_PQ_QueryParam params[] = { GNUNET_PQ_query_param_string (product_id), /* $1 */ @@ -93,6 +94,8 @@ TALER_MERCHANTDB_update_product ( no_group), GNUNET_PQ_result_spec_bool ("no_pot", no_pot), + GNUNET_PQ_result_spec_bool ("no_unit", + no_unit), GNUNET_PQ_result_spec_end }; enum GNUNET_DB_QueryStatus qs; @@ -116,6 +119,7 @@ TALER_MERCHANTDB_update_product ( ",out_no_cat AS no_cat" ",out_no_group AS no_group" ",out_no_pot AS no_pot" + ",out_no_unit AS no_unit" " FROM merchant_do_update_product" "($1,$2,$3::TEXT::JSONB,$4,$5,$6::TEXT::JSONB,$7,$8" ",$9,$10,$11,$12,$13::TEXT::JSONB,$14,$15,$16,$17" diff --git a/src/backenddb/update_product.sql b/src/backenddb/update_product.sql @@ -43,7 +43,8 @@ CREATE FUNCTION merchant_do_update_product ( OUT out_stocked_reduced BOOL, OUT out_no_cat INT8, OUT out_no_group BOOL, - OUT out_no_pot BOOL) + OUT out_no_pot BOOL, + OUT out_no_unit BOOL) LANGUAGE plpgsql AS $$ DECLARE @@ -55,12 +56,28 @@ BEGIN out_no_group = FALSE; out_no_pot = FALSE; +out_no_unit = FALSE; out_no_product=FALSE; out_lost_reduced=FALSE; out_sold_reduced=FALSE; -- We currently don't allow updating 'sold', hence always FALSE out_stocked_reduced=FALSE; out_no_cat=NULL; +-- Check the unit exists, as merchant_inventory.unit has no foreign key +-- into the unit tables. Before anything is modified. +PERFORM FROM merchant_custom_units + WHERE unit=in_unit; +IF NOT FOUND +THEN + PERFORM FROM merchant.merchant_builtin_units + WHERE unit=in_unit; + IF NOT FOUND + THEN + out_no_unit=TRUE; + RETURN; + END IF; +END IF; + IF in_product_group_id IS NOT NULL THEN PERFORM FROM merchant_product_groups diff --git a/src/include/merchant-database/insert_product.h b/src/include/merchant-database/insert_product.h @@ -39,6 +39,7 @@ * @param[out] no_cat set to index of non-existing category from @a cats, or -1 if all @a cats were found * @param[out] no_group set to true if the product group in @a pd is unknown * @param[out] no_pot set to true if the money pot in @a pd is unknown + * @param[out] no_unit set to true if the unit in @a pd is unknown * @return database result code */ enum GNUNET_DB_QueryStatus @@ -52,7 +53,8 @@ TALER_MERCHANTDB_insert_product ( bool *conflict, ssize_t *no_cat, bool *no_group, - bool *no_pot); + bool *no_pot, + bool *no_unit); #endif diff --git a/src/include/merchant-database/update_product.h b/src/include/merchant-database/update_product.h @@ -49,6 +49,7 @@ * the existing value; * @param[out] no_group set to true if the product group in @a pd is unknown * @param[out] no_pot set to true if the money pot in @a pd is unknown + * @param[out] no_unit set to true if the unit in @a pd is unknown * @return database result code */ enum GNUNET_DB_QueryStatus @@ -65,6 +66,7 @@ TALER_MERCHANTDB_update_product ( bool *sold_reduced, bool *stocked_reduced, bool *no_group, - bool *no_pot); + bool *no_pot, + bool *no_unit); #endif diff --git a/src/testing/test_merchant_api.c b/src/testing/test_merchant_api.c @@ -4455,6 +4455,64 @@ run (void *cls, merchant_payto, NULL, NULL, MHD_HTTP_OK), + /* The products created below are measured in units that are neither + builtin nor created anywhere else in this test; create them here, + in the instance that survives the purges above, as POST /products + and PATCH /products/$ID reject a unit that does not exist. + unit_allow_fraction is false and unit_precision_level is 0 for all + four on purpose: a POST/PATCH that omits those two fields inherits + them from the unit, and the product commands' traits expect the + false/0 that an *unknown* unit used to yield. The commands that do + exercise fractional stock ("post-products-frac" and friends) state + both fields explicitly, so the unit's policy does not reach them. */ + TALER_TESTING_cmd_merchant_post_units ( + "post-unit-test-unit", + merchant_url, + "test-unit", + "test unit", + "tu", + false, /* unit_allow_fraction */ + 0, /* unit_precision_level */ + true, /* unit_active */ + NULL, + NULL, + MHD_HTTP_NO_CONTENT), + TALER_TESTING_cmd_merchant_post_units ( + "post-unit-unit", + merchant_url, + "unit", + "generic unit", + "u", + false, /* unit_allow_fraction */ + 0, /* unit_precision_level */ + true, /* unit_active */ + NULL, + NULL, + MHD_HTTP_NO_CONTENT), + TALER_TESTING_cmd_merchant_post_units ( + "post-unit-kg", + merchant_url, + "kg", + "test kilogram", + "test-kg", + false, /* unit_allow_fraction */ + 0, /* unit_precision_level */ + true, /* unit_active */ + NULL, + NULL, + MHD_HTTP_NO_CONTENT), + TALER_TESTING_cmd_merchant_post_units ( + "post-unit-can", + merchant_url, + "can", + "can", + "can", + false, /* unit_allow_fraction */ + 0, /* unit_precision_level */ + true, /* unit_active */ + NULL, + NULL, + MHD_HTTP_NO_CONTENT), TALER_TESTING_cmd_merchant_get_products ( "get-products-empty", merchant_url, diff --git a/src/testing/test_merchant_order_creation.sh b/src/testing/test_merchant_order_creation.sh @@ -459,6 +459,23 @@ fi echo "OK" +# The products created below are measured in "u", which is not a builtin +# unit; POST/PATCH /private/products reject a unit that does not exist, so +# create it first. +echo -n "Creating unit for the products below ..." +STATUS=$(curl -H "Content-Type: application/json" -X POST \ + -H "Authorization: Bearer $ADMIN_TOKEN" \ + http://localhost:9966/private/units \ + -d '{"unit":"u","unit_name_long":"unit","unit_name_short":"u"}' \ + -w "%{http_code}" -s -o /dev/null) + +if [ "$STATUS" != "204" ] +then + exit_fail "Expected 204 No content, unit created. Got: $STATUS" +fi + +echo "OK" + # # CREATE INVENTORY PRODUCT AND CLAIM IT # diff --git a/src/testing/test_merchant_product_creation.sh b/src/testing/test_merchant_product_creation.sh @@ -88,6 +88,23 @@ fi echo "OK" +# The products created below are measured in "u", which is not a builtin +# unit; POST/PATCH /private/products reject a unit that does not exist, so +# create it first. +echo -n "Creating unit for the products below ..." +STATUS=$(curl -H "Content-Type: application/json" -X POST \ + -H "Authorization: Bearer $ADMIN_TOKEN" \ + http://localhost:9966/private/units \ + -d '{"unit":"u","unit_name_long":"unit","unit_name_short":"u"}' \ + -w "%{http_code}" -s -o /dev/null) + +if [ "$STATUS" != "204" ] +then + exit_fail "Expected 204 No content, unit created. Got: $STATUS" +fi + +echo "OK" + RANDOM_IMG='data:image/png;base64,abcdefg' INFINITE_PRODUCT_TEMPLATE='{"product_id":"2","product_name":"stuff","description":"product with id 2 and price :15","unit_price":["TESTKUDOS:15"],"unit_total_stock":"-1","unit":"u","image":"'"$RANDOM_IMG"'","taxes":[]}' MANAGED_PRODUCT_TEMPLATE='{"product_id":"3","product_name":"more stuff","description":"product with id 3 and price :10","unit_price":["TESTKUDOS:150"],"unit_total_stock":"2","unit":"u","image":"'"$RANDOM_IMG"'","taxes":[]}'