From 72caa08a326446ddf344b29e0bee773c67868e13 Mon Sep 17 00:00:00 2001 From: Bart Maertens Date: Wed, 7 Oct 2026 20:17:59 +0200 Subject: [PATCH 1/3] Issue #8791 : Harden the Mask fields database mapping store Store an HMAC of each source value when a pattern has a hash secret, and move plain-text rows to their hashed key. Reserve sequence numbers with a row lock and add a unique key on the masked value, so two processes never hand out the same value. Share a mapping store by resolved URL and user. Clear the failed and later fields of an error row. Add trim and ignore-case options and key numbers by value. Stop when a hash secret does not resolve. --- .../pages/metadata-types/masking-pattern.adoc | 11 +- .../pages/pipeline/transforms/maskfields.adoc | 14 +- .../0046-mask-fields-hashed-database.hpl | 186 +++++++++++ .../0047-mask-fields-plain-text-migration.hpl | 173 ++++++++++ .../main-0046-mask-fields-hashed-database.hwf | 312 ++++++++++++++++++ ...-0047-mask-fields-plain-text-migration.hwf | 234 +++++++++++++ .../masking-pattern/0046-first-name.json | 17 + .../masking-pattern/0047-first-name.json | 17 + .../script-mask-fields-hashed-database.sql | 27 ++ ...cript-mask-fields-plain-text-migration.sql | 43 +++ .../transforms/0120-mask-fields-normalize.hpl | 193 +++++++++++ .../golden-0120-mask-fields-normalize.csv | 5 + .../main-0120-mask-fields-normalize.hwf | 79 +++++ .../golden-0120-mask-fields-normalize.json | 40 +++ .../masking-pattern/0120-customer-nr.json | 17 + .../masking-pattern/0120-first-name.json | 17 + .../0120-mask-fields-normalize UNIT.json | 43 +++ .../transforms/maskfields/MaskFields.java | 5 +- .../transforms/maskfields/MaskFieldsMeta.java | 36 ++ .../transforms/maskfields/MaskingEngine.java | 49 ++- .../transforms/maskfields/MaskingKey.java | 154 +++++++++ .../transforms/maskfields/MaskingPattern.java | 43 +++ .../maskfields/MaskingPatternEditor.java | 11 +- .../transforms/maskfields/MaskingRuntime.java | 23 +- .../store/DatabaseMaskingStore.java | 160 ++++++--- .../maskfields/store/IMaskingStore.java | 18 + .../messages/messages_en_US.properties | 13 +- .../maskfields/MaskFieldsMetaTest.java | 57 ++++ .../maskfields/MaskingEngineTest.java | 45 ++- .../transforms/maskfields/MaskingKeyTest.java | 105 ++++++ .../store/DatabaseMaskingStoreTest.java | 169 +++++++++- 31 files changed, 2242 insertions(+), 74 deletions(-) create mode 100644 integration-tests/database/0046-mask-fields-hashed-database.hpl create mode 100644 integration-tests/database/0047-mask-fields-plain-text-migration.hpl create mode 100644 integration-tests/database/main-0046-mask-fields-hashed-database.hwf create mode 100644 integration-tests/database/main-0047-mask-fields-plain-text-migration.hwf create mode 100644 integration-tests/database/metadata/masking-pattern/0046-first-name.json create mode 100644 integration-tests/database/metadata/masking-pattern/0047-first-name.json create mode 100644 integration-tests/database/scripts/script-mask-fields-hashed-database.sql create mode 100644 integration-tests/database/scripts/script-mask-fields-plain-text-migration.sql create mode 100644 integration-tests/transforms/0120-mask-fields-normalize.hpl create mode 100644 integration-tests/transforms/datasets/golden-0120-mask-fields-normalize.csv create mode 100644 integration-tests/transforms/main-0120-mask-fields-normalize.hwf create mode 100644 integration-tests/transforms/metadata/dataset/golden-0120-mask-fields-normalize.json create mode 100644 integration-tests/transforms/metadata/masking-pattern/0120-customer-nr.json create mode 100644 integration-tests/transforms/metadata/masking-pattern/0120-first-name.json create mode 100644 integration-tests/transforms/metadata/unit-test/0120-mask-fields-normalize UNIT.json create mode 100644 plugins/transforms/maskfields/src/main/java/org/apache/hop/pipeline/transforms/maskfields/MaskingKey.java create mode 100644 plugins/transforms/maskfields/src/test/java/org/apache/hop/pipeline/transforms/maskfields/MaskingKeyTest.java diff --git a/docs/hop-user-manual/modules/ROOT/pages/metadata-types/masking-pattern.adoc b/docs/hop-user-manual/modules/ROOT/pages/metadata-types/masking-pattern.adoc index 9e569d3f856..dca8d136ab2 100644 --- a/docs/hop-user-manual/modules/ROOT/pages/metadata-types/masking-pattern.adoc +++ b/docs/hop-user-manual/modules/ROOT/pages/metadata-types/masking-pattern.adoc @@ -61,6 +61,12 @@ image::metadata-types/masking-pattern-first-name.jpg[Masking pattern first-name, |Remember values |*NONE* assigns a new value every row. *MEMORY* keeps the mapping for one run. *DATABASE* keeps it in a table across runs. +|Ignore surrounding spaces +|Values that only differ in leading or trailing spaces get the same replacement. A value of only spaces is left unchanged. Used by *MEMORY* and *DATABASE*. + +|Ignore case +|Values that only differ in upper and lower case get the same replacement. Used by *MEMORY* and *DATABASE*. + |Connection |Database connection for *DATABASE*. @@ -68,5 +74,8 @@ image::metadata-types/masking-pattern-first-name.jpg[Masking pattern first-name, |Schema of the mapping table. Empty uses the connection default. |Table -|Mapping table. A second table with the suffix `_seq` stores the next sequence number. Both are created when they are missing. The map table stores the original value and is itself personal data. +|Mapping table. A second table with the suffix `_seq` stores the next sequence number. Both are created when they are missing. A new mapping table holds masked values of up to 255 characters. + +|Hash secret +|The mapping table stores an HMAC-SHA256 of each value, made with this secret, instead of the value itself. Use a variable such as `${MASKING_SECRET}`. Changing the secret starts a new mapping. Without a secret the table stores the original values and is itself personal data. |=== diff --git a/docs/hop-user-manual/modules/ROOT/pages/pipeline/transforms/maskfields.adoc b/docs/hop-user-manual/modules/ROOT/pages/pipeline/transforms/maskfields.adoc index 4192848e1a8..09994a85207 100644 --- a/docs/hop-user-manual/modules/ROOT/pages/pipeline/transforms/maskfields.adoc +++ b/docs/hop-user-manual/modules/ROOT/pages/pipeline/transforms/maskfields.adoc @@ -55,6 +55,8 @@ image::transforms/maskfields/maskfields-dialog.png[Mask fields dialog,width="60% |Name of a Masking pattern metadata item. Leave this empty to skip the field. |=== +When a field cannot be masked and the transform has error handling, the error row has that field and every masked field after it set to null. + == What a pattern writes *SYNTHETIC* builds `prefix + token + suffix` for a string. The token is a *SEQUENCE* (an integer that starts at *Sequence start* and steps by one) or a *UUID*. A number field can take a sequence only, with an empty prefix and suffix. A date cannot take a generated token. @@ -67,11 +69,17 @@ image::transforms/maskfields/maskfields-dialog.png[Mask fields dialog,width="60% *MEMORY* keeps the mapping for this pipeline execution. The next run starts again, so tokens from the previous run can be issued again. -*DATABASE* keeps the mapping in the table named on the pattern. A second table, that name plus `_seq`, stores the next sequence number. Both tables are created when they are missing. The map table has `pattern_name`, `source_key` and `masked_value`, with a primary key on the pattern name and the source key. +*DATABASE* keeps the mapping in the table named on the pattern. A second table, that name plus `_seq`, stores the next sequence number. Both tables are created when they are missing. The map table has `pattern_name`, `source_key` and `masked_value`, with a primary key on the pattern name and the source key. A new map table also has a unique key on the pattern name and the masked value, and holds masked values of up to 255 characters. + +Give a database pattern a *Hash secret*. The map table then stores an HMAC-SHA256 of each value, made with that secret, instead of the value itself. Without a secret, `source_key` holds the original value, so the table is itself personal data, and the transform check warns about it. Keep the secret in a variable, out of the project. Changing the secret starts a new mapping. A row stored in plain text before the pattern had a secret is moved to its hashed key the next time its value is masked. Rows for values that do not come by again stay in plain text, so delete those once the mapping has been rebuilt. + +Several Hop processes can share one map table. Each new sequence number is reserved with a row lock, so two processes never hand out the same value. + +*Ignore surrounding spaces* and *Ignore case* make values that only differ in those ways share a replacement. A number is compared by its value, not by its conversion mask. Turning these options on for a database pattern that already holds mappings changes replacements: spellings such as `Matt` and `matt` that had their own replacement now share the one stored for whichever spelling comes by first, so data masked earlier no longer matches for the others. -The map table stores the original value, so it is itself personal data. Put it on a connection you protect, separate from the masked output. +A hash secret that resolves to nothing, or still holds a variable that is not set, stops the transform from starting. It never falls back to plain text or to the literal text of the variable. -Copies and partitions of one pipeline share one runtime, so a remembered value stays the same and a sequence does not start over on each copy. Which row receives which new sequence number follows whichever copy runs first. Beam and native Spark each run a worker in its own JVM, so they stay unsupported. The database store is what shares a mapping across pipelines. +Copies and partitions of one pipeline share one runtime, so a remembered value stays the same and a sequence does not start over on each copy. Which row receives which new sequence number follows whichever copy runs first. Beam and native Spark each run a worker in its own JVM, so they stay unsupported. The database store is what shares a mapping across pipelines. Copies share a database store when its connection resolves to the same URL and user. == Related diff --git a/integration-tests/database/0046-mask-fields-hashed-database.hpl b/integration-tests/database/0046-mask-fields-hashed-database.hpl new file mode 100644 index 00000000000..0ac4b4050b2 --- /dev/null +++ b/integration-tests/database/0046-mask-fields-hashed-database.hpl @@ -0,0 +1,186 @@ + + + + + 0046-mask-fields-hashed-database + Y + + + + Normal + + + RESULT_TABLE + mask_0046_run1 + Table the masked rows are written to + + + N + 1000 + 100 + - + 2026/10/07 20:00:00.000 + - + 2026/10/07 20:00:00.000 + + + + + + people + Mask fields + Y + + + Mask fields + result + Y + + + + people + DataGrid + + Y + + 1 + + none + + + + + + + + id + Integer + + -1 + -1 + N + + + + + + firstName + String + + -1 + -1 + N + + + + + 1 + Matt + + + 2 + Ann + + + 3 + Bart + + + 4 + Matt + + + + + 96 + 96 + + + + Mask fields + MaskFields + + Y + + 1 + + none + + + + + firstName + 0046-first-name + + + + + 272 + 96 + + + + result + TableOutput + + Y + + 1 + + none + + + 1000 + unit-test-db + + + id + id + + + first_name + firstName + + + + N + N + N + + Y + N + public + Y + ${RESULT_TABLE}
+ + N + Y + Y + Y + + + 448 + 96 + + + + + + diff --git a/integration-tests/database/0047-mask-fields-plain-text-migration.hpl b/integration-tests/database/0047-mask-fields-plain-text-migration.hpl new file mode 100644 index 00000000000..639f7f24857 --- /dev/null +++ b/integration-tests/database/0047-mask-fields-plain-text-migration.hpl @@ -0,0 +1,173 @@ + + + + + 0047-mask-fields-plain-text-migration + Y + + + + Normal + + + N + 1000 + 100 + - + 2026/10/07 20:00:00.000 + - + 2026/10/07 20:00:00.000 + + + + + + people + Mask fields + Y + + + Mask fields + result + Y + + + + people + DataGrid + + Y + + 1 + + none + + + + + + + + id + Integer + + -1 + -1 + N + + + + + + firstName + String + + -1 + -1 + N + + + + + 1 + Matt + + + 2 + Ann + + + + + 96 + 96 + + + + Mask fields + MaskFields + + Y + + 1 + + none + + + + + firstName + 0047-first-name + + + + + 272 + 96 + + + + result + TableOutput + + Y + + 1 + + none + + + 1000 + unit-test-db + + + id + id + + + first_name + firstName + + + + N + N + N + + Y + N + public + Y + mask_0047_result
+ + N + Y + Y + Y + + + 448 + 96 + + + + + + diff --git a/integration-tests/database/main-0046-mask-fields-hashed-database.hwf b/integration-tests/database/main-0046-mask-fields-hashed-database.hwf new file mode 100644 index 00000000000..4bdaec24161 --- /dev/null +++ b/integration-tests/database/main-0046-mask-fields-hashed-database.hwf @@ -0,0 +1,312 @@ + + + + main-0046-mask-fields-hashed-database + Y + + + + - + 2026/10/07 20:00:00.000 + - + 2026/10/07 20:00:00.000 + + + + + Start + + SPECIAL + + N + 0 + 0 + 60 + 12 + 0 + 1 + 1 + N + 64 + 96 + + + + scripts/script-mask-fields-hashed-database.sql + + SQL + unit-test-db + N + + ${PROJECT_HOME}/scripts/script-mask-fields-hashed-database.sql + Y + N + + N + 224 + 96 + + + + first run + + PIPELINE + N + N + N + N + N + N + ${PROJECT_HOME}/0046-mask-fields-hashed-database.hpl + + + Basic + + + RESULT_TABLE + + mask_0046_run1 + + Y + + N + local + N + N + Y + + N + 384 + 96 + + + + second run + + PIPELINE + N + N + N + N + N + N + ${PROJECT_HOME}/0046-mask-fields-hashed-database.hpl + + + Basic + + + RESULT_TABLE + + mask_0046_run2 + + Y + + N + local + N + N + Y + + N + 544 + 96 + + + + 3 hashed mappings + + EVAL_TABLE_CONTENT + N + Y + unit-test-db + SELECT 1 +FROM public.mask_0046 +WHERE source_key LIKE 'hmac-sha256:%' + + Y + N + 3 + public + rows_count_equal + + + N + 704 + 96 + + + + no names in plain text + + EVAL_TABLE_CONTENT + N + Y + unit-test-db + SELECT 1 +FROM public.mask_0046 +WHERE source_key IN ('Matt', 'Ann', 'Bart') + + Y + N + 0 + public + rows_count_equal + + + N + 864 + 96 + + + + no masked value used twice + + EVAL_TABLE_CONTENT + N + Y + unit-test-db + SELECT masked_value +FROM public.mask_0046 +GROUP BY masked_value +HAVING COUNT(*) > 1 + + Y + N + 0 + public + rows_count_equal + + + N + 1024 + 96 + + + + Matt keeps one value + + EVAL_TABLE_CONTENT + N + Y + unit-test-db + SELECT DISTINCT first_name +FROM public.mask_0046_run1 +WHERE id IN (1, 4) + + Y + N + 1 + public + rows_count_equal + + + N + 1184 + 96 + + + + both runs give the same values + + EVAL_TABLE_CONTENT + N + Y + unit-test-db + SELECT 1 +FROM public.mask_0046_run1 r1 +JOIN public.mask_0046_run2 r2 ON r1.id = r2.id AND r1.first_name = r2.first_name +WHERE r1.first_name LIKE 'fn%' + + Y + N + 4 + public + rows_count_equal + + + N + 1344 + 96 + + + + + + Start + scripts/script-mask-fields-hashed-database.sql + Y + Y + Y + + + scripts/script-mask-fields-hashed-database.sql + first run + Y + Y + N + + + first run + second run + Y + Y + N + + + second run + 3 hashed mappings + Y + Y + N + + + 3 hashed mappings + no names in plain text + Y + Y + N + + + no names in plain text + no masked value used twice + Y + Y + N + + + no masked value used twice + Matt keeps one value + Y + Y + N + + + Matt keeps one value + both runs give the same values + Y + Y + N + + + + + + diff --git a/integration-tests/database/main-0047-mask-fields-plain-text-migration.hwf b/integration-tests/database/main-0047-mask-fields-plain-text-migration.hwf new file mode 100644 index 00000000000..7d5cda22e11 --- /dev/null +++ b/integration-tests/database/main-0047-mask-fields-plain-text-migration.hwf @@ -0,0 +1,234 @@ + + + + main-0047-mask-fields-plain-text-migration + Y + + + + - + 2026/10/07 20:00:00.000 + - + 2026/10/07 20:00:00.000 + + + + + Start + + SPECIAL + + N + 0 + 0 + 60 + 12 + 0 + 1 + 1 + N + 64 + 96 + + + + scripts/script-mask-fields-plain-text-migration.sql + + SQL + unit-test-db + N + + ${PROJECT_HOME}/scripts/script-mask-fields-plain-text-migration.sql + Y + N + + N + 224 + 96 + + + + mask + + PIPELINE + N + N + N + N + N + N + ${PROJECT_HOME}/0047-mask-fields-plain-text-migration.hpl + + + Basic + + Y + + N + local + N + N + Y + + N + 384 + 96 + + + + Matt keeps the stored value + + EVAL_TABLE_CONTENT + N + Y + unit-test-db + SELECT 1 +FROM public.mask_0047_result +WHERE id = 1 AND first_name = 'fn-legacy' + + Y + N + 1 + public + rows_count_equal + + + N + 544 + 96 + + + + Ann continues the stored sequence + + EVAL_TABLE_CONTENT + N + Y + unit-test-db + SELECT 1 +FROM public.mask_0047_result +WHERE id = 2 AND first_name = 'fn100' + + Y + N + 1 + public + rows_count_equal + + + N + 704 + 96 + + + + 2 mappings + + EVAL_TABLE_CONTENT + N + Y + unit-test-db + SELECT 1 +FROM public.mask_0047 + + Y + N + 2 + public + rows_count_equal + + + N + 864 + 96 + + + + no keys left in plain text + + EVAL_TABLE_CONTENT + N + Y + unit-test-db + SELECT 1 +FROM public.mask_0047 +WHERE source_key NOT LIKE 'hmac-sha256:%' + + Y + N + 0 + public + rows_count_equal + + + N + 1024 + 96 + + + + + + Start + scripts/script-mask-fields-plain-text-migration.sql + Y + Y + Y + + + scripts/script-mask-fields-plain-text-migration.sql + mask + Y + Y + N + + + mask + Matt keeps the stored value + Y + Y + N + + + Matt keeps the stored value + Ann continues the stored sequence + Y + Y + N + + + Ann continues the stored sequence + 2 mappings + Y + Y + N + + + 2 mappings + no keys left in plain text + Y + Y + N + + + + + + diff --git a/integration-tests/database/metadata/masking-pattern/0046-first-name.json b/integration-tests/database/metadata/masking-pattern/0046-first-name.json new file mode 100644 index 00000000000..c43b71c7d96 --- /dev/null +++ b/integration-tests/database/metadata/masking-pattern/0046-first-name.json @@ -0,0 +1,17 @@ +{ + "name": "0046-first-name", + "description": "Database mapping with a hash secret", + "piiClassification": "Direct identifier", + "valueSource": "SYNTHETIC", + "token": "SEQUENCE", + "prefix": "fn", + "suffix": "", + "sequenceStart": "1", + "storage": "DATABASE", + "trimKey": false, + "ignoreCase": false, + "connection": "unit-test-db", + "schemaName": "public", + "tableName": "mask_0046", + "hashSecret": "it-0046-secret" +} diff --git a/integration-tests/database/metadata/masking-pattern/0047-first-name.json b/integration-tests/database/metadata/masking-pattern/0047-first-name.json new file mode 100644 index 00000000000..fde266860a7 --- /dev/null +++ b/integration-tests/database/metadata/masking-pattern/0047-first-name.json @@ -0,0 +1,17 @@ +{ + "name": "0047-first-name", + "description": "Database mapping with a hash secret on a table that holds plain-text keys", + "piiClassification": "Direct identifier", + "valueSource": "SYNTHETIC", + "token": "SEQUENCE", + "prefix": "fn", + "suffix": "", + "sequenceStart": "1", + "storage": "DATABASE", + "trimKey": false, + "ignoreCase": false, + "connection": "unit-test-db", + "schemaName": "public", + "tableName": "mask_0047", + "hashSecret": "it-0047-secret" +} diff --git a/integration-tests/database/scripts/script-mask-fields-hashed-database.sql b/integration-tests/database/scripts/script-mask-fields-hashed-database.sql new file mode 100644 index 00000000000..3ea48faa490 --- /dev/null +++ b/integration-tests/database/scripts/script-mask-fields-hashed-database.sql @@ -0,0 +1,27 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * + */ + +/* Issue #8791: Mask fields stores an HMAC of each value, not the value itself. */ + +DROP TABLE IF EXISTS public.mask_0046; +DROP TABLE IF EXISTS public.mask_0046_seq; +DROP TABLE IF EXISTS public.mask_0046_run1; +DROP TABLE IF EXISTS public.mask_0046_run2; + +CREATE TABLE public.mask_0046_run1 (id INTEGER, first_name VARCHAR(100)); +CREATE TABLE public.mask_0046_run2 (id INTEGER, first_name VARCHAR(100)); diff --git a/integration-tests/database/scripts/script-mask-fields-plain-text-migration.sql b/integration-tests/database/scripts/script-mask-fields-plain-text-migration.sql new file mode 100644 index 00000000000..406d297984d --- /dev/null +++ b/integration-tests/database/scripts/script-mask-fields-plain-text-migration.sql @@ -0,0 +1,43 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * + */ + +/* + * Issue #8791: a mapping table written before patterns had a hash secret. It has the old layout and + * stores the source value in plain text. Masking with a secret keeps the replacement and moves the + * row to its hashed key. + */ + +DROP TABLE IF EXISTS public.mask_0047; +DROP TABLE IF EXISTS public.mask_0047_seq; +DROP TABLE IF EXISTS public.mask_0047_result; + +CREATE TABLE public.mask_0047 ( + pattern_name VARCHAR(128) NOT NULL, + source_key VARCHAR(255) NOT NULL, + masked_value VARCHAR(2000) NOT NULL, + PRIMARY KEY (pattern_name, source_key) +); +CREATE TABLE public.mask_0047_seq ( + pattern_name VARCHAR(128) NOT NULL, + next_value BIGINT NOT NULL, + PRIMARY KEY (pattern_name) +); +CREATE TABLE public.mask_0047_result (id INTEGER, first_name VARCHAR(100)); + +INSERT INTO public.mask_0047 VALUES ('0047-first-name', 'Matt', 'fn-legacy'); +INSERT INTO public.mask_0047_seq VALUES ('0047-first-name', 100); diff --git a/integration-tests/transforms/0120-mask-fields-normalize.hpl b/integration-tests/transforms/0120-mask-fields-normalize.hpl new file mode 100644 index 00000000000..46c4afc85e4 --- /dev/null +++ b/integration-tests/transforms/0120-mask-fields-normalize.hpl @@ -0,0 +1,193 @@ + + + + + 0120-mask-fields-normalize + Y + + + + Normal + + + N + 1000 + 100 + - + 2026/10/07 20:00:00.000 + - + 2026/10/07 20:00:00.000 + + + + + + people + Mask fields + Y + + + Mask fields + Result + Y + + + + people + DataGrid + + Y + + 1 + + none + + + + + + + + id + Integer + + -1 + -1 + N + + + + + + firstName + String + + -1 + -1 + N + + + + + + customerNr + Integer + 000000 + -1 + -1 + N + + + + + + customerNrPlain + Integer + + -1 + -1 + N + + + + + 1 + Matt + 000042 + 42 + + + 2 + MATT + 000007 + 7 + + + 3 + matt + 000042 + 42 + + + 4 + Ann + 001001 + 1001 + + + + + 96 + 96 + + + + Mask fields + MaskFields + + Y + + 1 + + none + + + + + firstName + 0120-first-name + + + customerNr + 0120-customer-nr + + + customerNrPlain + 0120-customer-nr + + + + + 272 + 96 + + + + Result + Dummy + + Y + + 1 + + none + + + + + + 448 + 96 + + + + + + diff --git a/integration-tests/transforms/datasets/golden-0120-mask-fields-normalize.csv b/integration-tests/transforms/datasets/golden-0120-mask-fields-normalize.csv new file mode 100644 index 00000000000..85f4ba722b3 --- /dev/null +++ b/integration-tests/transforms/datasets/golden-0120-mask-fields-normalize.csv @@ -0,0 +1,5 @@ +id,firstName,customerNr,customerNrPlain +1,fn1,500,500 +2,fn1,501,501 +3,fn1,500,500 +4,fn2,502,502 diff --git a/integration-tests/transforms/main-0120-mask-fields-normalize.hwf b/integration-tests/transforms/main-0120-mask-fields-normalize.hwf new file mode 100644 index 00000000000..49d3714a50d --- /dev/null +++ b/integration-tests/transforms/main-0120-mask-fields-normalize.hwf @@ -0,0 +1,79 @@ + + + + main-0120-mask-fields-normalize + Y + + + + - + 2026/10/07 20:00:00.000 + - + 2026/10/07 20:00:00.000 + + + + + Start + + SPECIAL + + N + 0 + 0 + 60 + 12 + 0 + 1 + 1 + N + 64 + 96 + + + + Run Mask Fields normalize test + + RunPipelineTests + + + 0120-mask-fields-normalize UNIT + + + + N + 224 + 96 + + + + + + Start + Run Mask Fields normalize test + Y + Y + Y + + + + + + diff --git a/integration-tests/transforms/metadata/dataset/golden-0120-mask-fields-normalize.json b/integration-tests/transforms/metadata/dataset/golden-0120-mask-fields-normalize.json new file mode 100644 index 00000000000..982f2ee2e03 --- /dev/null +++ b/integration-tests/transforms/metadata/dataset/golden-0120-mask-fields-normalize.json @@ -0,0 +1,40 @@ +{ + "base_filename": "golden-0120-mask-fields-normalize.csv", + "name": "golden-0120-mask-fields-normalize", + "description": "Masked values from 0120-mask-fields-normalize", + "dataset_fields": [ + { + "field_comment": "", + "field_length": -1, + "field_type": 5, + "field_precision": -1, + "field_format": "", + "field_name": "id" + }, + { + "field_comment": "", + "field_length": -1, + "field_type": 2, + "field_precision": -1, + "field_format": "", + "field_name": "firstName" + }, + { + "field_comment": "", + "field_length": -1, + "field_type": 5, + "field_precision": -1, + "field_format": "", + "field_name": "customerNr" + }, + { + "field_comment": "", + "field_length": -1, + "field_type": 5, + "field_precision": -1, + "field_format": "", + "field_name": "customerNrPlain" + } + ], + "folder_name": "" +} diff --git a/integration-tests/transforms/metadata/masking-pattern/0120-customer-nr.json b/integration-tests/transforms/metadata/masking-pattern/0120-customer-nr.json new file mode 100644 index 00000000000..45f688f9dbc --- /dev/null +++ b/integration-tests/transforms/metadata/masking-pattern/0120-customer-nr.json @@ -0,0 +1,17 @@ +{ + "name": "0120-customer-nr", + "description": "Shared by two integer fields with different conversion masks", + "piiClassification": "Quasi-identifier", + "valueSource": "SYNTHETIC", + "token": "SEQUENCE", + "prefix": "", + "suffix": "", + "sequenceStart": "500", + "storage": "MEMORY", + "trimKey": false, + "ignoreCase": false, + "connection": "", + "schemaName": "", + "tableName": "", + "hashSecret": "" +} diff --git a/integration-tests/transforms/metadata/masking-pattern/0120-first-name.json b/integration-tests/transforms/metadata/masking-pattern/0120-first-name.json new file mode 100644 index 00000000000..6f525c8ef21 --- /dev/null +++ b/integration-tests/transforms/metadata/masking-pattern/0120-first-name.json @@ -0,0 +1,17 @@ +{ + "name": "0120-first-name", + "description": "Ignores surrounding spaces and case", + "piiClassification": "Direct identifier", + "valueSource": "SYNTHETIC", + "token": "SEQUENCE", + "prefix": "fn", + "suffix": "", + "sequenceStart": "1", + "storage": "MEMORY", + "trimKey": true, + "ignoreCase": true, + "connection": "", + "schemaName": "", + "tableName": "", + "hashSecret": "" +} diff --git a/integration-tests/transforms/metadata/unit-test/0120-mask-fields-normalize UNIT.json b/integration-tests/transforms/metadata/unit-test/0120-mask-fields-normalize UNIT.json new file mode 100644 index 00000000000..ea1c2cad491 --- /dev/null +++ b/integration-tests/transforms/metadata/unit-test/0120-mask-fields-normalize UNIT.json @@ -0,0 +1,43 @@ +{ + "database_replacements": [], + "autoOpening": false, + "description": "Spaces, case and conversion masks do not change a remembered replacement", + "persist_filename": "", + "test_type": "UNIT_TEST", + "variableValues": [], + "basePath": "", + "golden_data_sets": [ + { + "field_mappings": [ + { + "transform_field": "id", + "data_set_field": "id" + }, + { + "transform_field": "firstName", + "data_set_field": "firstName" + }, + { + "transform_field": "customerNr", + "data_set_field": "customerNr" + }, + { + "transform_field": "customerNrPlain", + "data_set_field": "customerNrPlain" + } + ], + "field_order": [ + "id", + "firstName", + "customerNr", + "customerNrPlain" + ], + "data_set_name": "golden-0120-mask-fields-normalize", + "transform_name": "Result" + } + ], + "input_data_sets": [], + "name": "0120-mask-fields-normalize UNIT", + "trans_test_tweaks": [], + "pipeline_filename": "./0120-mask-fields-normalize.hpl" +} diff --git a/plugins/transforms/maskfields/src/main/java/org/apache/hop/pipeline/transforms/maskfields/MaskFields.java b/plugins/transforms/maskfields/src/main/java/org/apache/hop/pipeline/transforms/maskfields/MaskFields.java index 33fba597ccd..860a78a16f8 100644 --- a/plugins/transforms/maskfields/src/main/java/org/apache/hop/pipeline/transforms/maskfields/MaskFields.java +++ b/plugins/transforms/maskfields/src/main/java/org/apache/hop/pipeline/transforms/maskfields/MaskFields.java @@ -76,6 +76,8 @@ public boolean init() { patterns.put(field.getPatternName(), pattern); } long start = parseStart(pattern); + // Check the hash secret before the mapping table is opened. + MaskingKey key = MaskingKey.forPattern(pattern, this); IMaskingStore store = storeFor(pattern, lease); bindings.add( new Binding( @@ -85,7 +87,8 @@ public boolean init() { resolve(pattern.getSuffix()), start, store, - sharedSequence(lease, transformName, field, pattern, start))); + sharedSequence(lease, transformName, field, pattern, start), + key)); } } data.engine = new MaskingEngine(bindings); diff --git a/plugins/transforms/maskfields/src/main/java/org/apache/hop/pipeline/transforms/maskfields/MaskFieldsMeta.java b/plugins/transforms/maskfields/src/main/java/org/apache/hop/pipeline/transforms/maskfields/MaskFieldsMeta.java index 9f182e3e134..3432f4e1dec 100644 --- a/plugins/transforms/maskfields/src/main/java/org/apache/hop/pipeline/transforms/maskfields/MaskFieldsMeta.java +++ b/plugins/transforms/maskfields/src/main/java/org/apache/hop/pipeline/transforms/maskfields/MaskFieldsMeta.java @@ -45,6 +45,7 @@ import org.apache.hop.pipeline.PipelineMeta; import org.apache.hop.pipeline.transform.BaseTransformMeta; import org.apache.hop.pipeline.transform.TransformMeta; +import org.apache.hop.pipeline.transforms.maskfields.store.DatabaseMaskingStore; @Getter @Setter @@ -69,6 +70,11 @@ public class MaskFieldsMeta extends BaseTransformMeta DatabaseMaskingStore.MASKED_LENGTH) { + warning( + remarks, + transformMeta, + "MaskFields.Check.MaskedTooLong", + pattern.getName(), + Integer.toString(longest), + Integer.toString(DatabaseMaskingStore.MASKED_LENGTH)); + } if (metadataProvider == null) { return; } @@ -236,6 +258,20 @@ private MaskingPattern loadQuietly(IHopMetadataProvider metadataProvider, String } } + private static String resolved(IVariables variables, String value) { + String text = variables == null ? value : variables.resolve(value); + return text == null ? "" : text; + } + + private void warning( + List remarks, TransformMeta transformMeta, String key, String... args) { + remarks.add( + new CheckResult( + ICheckResult.TYPE_RESULT_WARNING, + BaseMessages.getString(PKG, key, (Object[]) args), + transformMeta)); + } + private void error( List remarks, TransformMeta transformMeta, String key, String... args) { remarks.add( diff --git a/plugins/transforms/maskfields/src/main/java/org/apache/hop/pipeline/transforms/maskfields/MaskingEngine.java b/plugins/transforms/maskfields/src/main/java/org/apache/hop/pipeline/transforms/maskfields/MaskingEngine.java index e92020defc4..a54c9b49731 100644 --- a/plugins/transforms/maskfields/src/main/java/org/apache/hop/pipeline/transforms/maskfields/MaskingEngine.java +++ b/plugins/transforms/maskfields/src/main/java/org/apache/hop/pipeline/transforms/maskfields/MaskingEngine.java @@ -45,10 +45,17 @@ public List getBindings() { return bindings; } + /** + * Masks the bound fields of the row in place. When a field fails, that field and every bound + * field after it are set to null before the error is thrown, so an error row never carries an + * original value. + */ public void apply(IRowMeta rowMeta, Object[] row) throws HopException { - for (Binding binding : bindings) { + for (int i = 0; i < bindings.size(); i++) { + Binding binding = bindings.get(i); int index = rowMeta.indexOfValue(binding.fieldName); if (index < 0) { + clearFrom(rowMeta, row, i + 1); throw new HopException( BaseMessages.getString(PKG, "MaskFields.Error.FieldMissing", binding.fieldName)); } @@ -56,10 +63,23 @@ public void apply(IRowMeta rowMeta, Object[] row) throws HopException { try { row[index] = mask(binding, valueMeta, row[index]); } catch (HopValueException e) { + clearFrom(rowMeta, row, i); throw new HopException( BaseMessages.getString( PKG, "MaskFields.Error.Convert", binding.fieldName, binding.pattern.getName()), e); + } catch (HopException | RuntimeException e) { + clearFrom(rowMeta, row, i); + throw e; + } + } + } + + private void clearFrom(IRowMeta rowMeta, Object[] row, int first) { + for (int i = first; i < bindings.size(); i++) { + int index = rowMeta.indexOfValue(bindings.get(i).fieldName); + if (index >= 0 && index < row.length) { + row[index] = null; } } } @@ -78,21 +98,22 @@ private Object mask(Binding binding, IValueMeta valueMeta, Object value) if (source == MaskingValueSource.SET_EMPTY) { return ""; } - String key = valueMeta.getString(value); - if (StringUtils.isEmpty(key)) { + String normalized = binding.key.normalize(valueMeta, value); + if (StringUtils.isEmpty(normalized)) { return value; } - return toField(valueMeta, syntheticValue(binding, valueMeta, key)); + return toField(valueMeta, syntheticValue(binding, valueMeta, value, normalized)); } - private String syntheticValue(Binding binding, IValueMeta valueMeta, String key) - throws HopException { + private String syntheticValue( + Binding binding, IValueMeta valueMeta, Object value, String normalized) throws HopException { if (!binding.pattern.remembers()) { return formatSynthetic(binding, valueMeta, nextToken(binding)); } return binding.store.findOrCreate( binding.pattern.getName(), - key, + binding.key.storeKey(normalized), + () -> MaskingKey.legacyKey(valueMeta, value), store -> formatSynthetic(binding, valueMeta, nextToken(binding, store))); } @@ -136,6 +157,7 @@ public static final class Binding { final IMaskingStore store; final AtomicLong localSequence; final AtomicLong sharedSequence; + final MaskingKey key; public Binding( String fieldName, @@ -155,6 +177,18 @@ public Binding( long sequenceStart, IMaskingStore store, AtomicLong sharedSequence) { + this(fieldName, pattern, prefix, suffix, sequenceStart, store, sharedSequence, null); + } + + public Binding( + String fieldName, + MaskingPattern pattern, + String prefix, + String suffix, + long sequenceStart, + IMaskingStore store, + AtomicLong sharedSequence, + MaskingKey key) { this.fieldName = fieldName; this.pattern = pattern; this.prefix = prefix == null ? "" : prefix; @@ -163,6 +197,7 @@ public Binding( this.store = store; this.sharedSequence = sharedSequence; this.localSequence = sharedSequence == null ? new AtomicLong(sequenceStart) : null; + this.key = key == null ? MaskingKey.plain() : key; } } } diff --git a/plugins/transforms/maskfields/src/main/java/org/apache/hop/pipeline/transforms/maskfields/MaskingKey.java b/plugins/transforms/maskfields/src/main/java/org/apache/hop/pipeline/transforms/maskfields/MaskingKey.java new file mode 100644 index 00000000000..8f31a0aca6e --- /dev/null +++ b/plugins/transforms/maskfields/src/main/java/org/apache/hop/pipeline/transforms/maskfields/MaskingKey.java @@ -0,0 +1,154 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.hop.pipeline.transforms.maskfields; + +import java.math.BigDecimal; +import java.nio.charset.StandardCharsets; +import java.security.GeneralSecurityException; +import java.util.HexFormat; +import java.util.Locale; +import javax.crypto.Mac; +import javax.crypto.spec.SecretKeySpec; +import org.apache.commons.lang3.StringUtils; +import org.apache.hop.core.encryption.Encr; +import org.apache.hop.core.exception.HopException; +import org.apache.hop.core.exception.HopValueException; +import org.apache.hop.core.row.IValueMeta; +import org.apache.hop.core.util.StringUtil; +import org.apache.hop.core.variables.IVariables; +import org.apache.hop.i18n.BaseMessages; + +/** + * Builds the key a remembering pattern finds a source value by. The key ignores the field's + * conversion mask, can ignore spaces around the value and its case, and is an HMAC of the value + * when the pattern has a hash secret. + */ +public final class MaskingKey { + + private static final Class PKG = MaskingKey.class; + + static final String HMAC_PREFIX = "hmac-sha256:"; + private static final String ALGORITHM = "HmacSHA256"; + + private final boolean trim; + private final boolean ignoreCase; + private final Mac mac; + + public MaskingKey(boolean trim, boolean ignoreCase, String secret) throws HopException { + this.trim = trim; + this.ignoreCase = ignoreCase; + if (StringUtils.isEmpty(secret)) { + this.mac = null; + } else { + try { + Mac hmac = Mac.getInstance(ALGORITHM); + hmac.init(new SecretKeySpec(secret.getBytes(StandardCharsets.UTF_8), ALGORITHM)); + this.mac = hmac; + } catch (GeneralSecurityException e) { + throw new HopException("Unable to initialize " + ALGORITHM, e); + } + } + } + + /** + * The key for a pattern. Only a database pattern hashes its keys. A hash secret that resolves to + * nothing, or still holds a variable, is an error: hashing with the literal text or falling back + * to plain text would both go unnoticed. + */ + public static MaskingKey forPattern(MaskingPattern pattern, IVariables variables) + throws HopException { + String secret = null; + if (pattern.getStorage() == MaskingStorage.DATABASE + && StringUtils.isNotEmpty(pattern.getHashSecret())) { + String resolved = variables.resolve(pattern.getHashSecret()); + if (StringUtils.isEmpty(resolved) || StringUtil.containsVariableToken(resolved)) { + throw new HopException( + BaseMessages.getString( + PKG, "MaskFields.Error.HashSecretUnresolved", pattern.getName())); + } + secret = Encr.decryptPasswordOptionallyEncrypted(resolved); + } + return new MaskingKey(pattern.isTrimKey(), pattern.isIgnoreCase(), secret); + } + + /** Plain key: no trimming, case-sensitive and not hashed. */ + public static MaskingKey plain() { + try { + return new MaskingKey(false, false, null); + } catch (HopException e) { + throw new IllegalStateException(e); + } + } + + /** + * @return the value as the pattern compares it, or null or empty when there is nothing to mask + */ + public String normalize(IValueMeta valueMeta, Object value) throws HopValueException { + if (value == null) { + return null; + } + String text = canonical(valueMeta, value); + if (text == null) { + return null; + } + if (trim) { + text = text.trim(); + } + if (ignoreCase) { + text = text.toLowerCase(Locale.ROOT); + } + return text; + } + + /** The key stored in a mapping, built from {@link #normalize(IValueMeta, Object)}. */ + public String storeKey(String normalized) { + if (mac == null) { + return normalized; + } + byte[] digest; + synchronized (mac) { + digest = mac.doFinal(normalized.getBytes(StandardCharsets.UTF_8)); + } + return HMAC_PREFIX + HexFormat.of().formatHex(digest); + } + + /** + * The key earlier versions stored: the value as the field formats it, in plain text. A mapping + * table can still hold rows with that key. + */ + public static String legacyKey(IValueMeta valueMeta, Object value) throws HopValueException { + return valueMeta.getString(value); + } + + private static String canonical(IValueMeta valueMeta, Object value) throws HopValueException { + if (!valueMeta.isNumeric()) { + return valueMeta.getString(value); + } + Object number = valueMeta.getNativeDataType(value); + if (number instanceof Long longValue) { + return Long.toString(longValue); + } + if (number instanceof Double doubleValue && Double.isFinite(doubleValue)) { + return BigDecimal.valueOf(doubleValue).stripTrailingZeros().toPlainString(); + } + if (number instanceof BigDecimal bigDecimal) { + return bigDecimal.stripTrailingZeros().toPlainString(); + } + return valueMeta.getString(value); + } +} diff --git a/plugins/transforms/maskfields/src/main/java/org/apache/hop/pipeline/transforms/maskfields/MaskingPattern.java b/plugins/transforms/maskfields/src/main/java/org/apache/hop/pipeline/transforms/maskfields/MaskingPattern.java index cc6c539be2d..d5320cd59f1 100644 --- a/plugins/transforms/maskfields/src/main/java/org/apache/hop/pipeline/transforms/maskfields/MaskingPattern.java +++ b/plugins/transforms/maskfields/src/main/java/org/apache/hop/pipeline/transforms/maskfields/MaskingPattern.java @@ -57,6 +57,9 @@ public class MaskingPattern extends HopMetadataBase implements IHopMetadata { public static final String WIDGET_SUFFIX = "suffix"; public static final String WIDGET_SEQUENCE_START = "sequenceStart"; public static final String WIDGET_STORAGE = "storage"; + public static final String WIDGET_TRIM_KEY = "trimKey"; + public static final String WIDGET_IGNORE_CASE = "ignoreCase"; + public static final String WIDGET_HASH_SECRET = "hashSecret"; public static final String WIDGET_CONNECTION = "connection"; public static final String WIDGET_SCHEMA = "schemaName"; public static final String WIDGET_TABLE = "tableName"; @@ -175,6 +178,32 @@ public class MaskingPattern extends HopMetadataBase implements IHopMetadata { toolTip = "i18n::MaskingPattern.Storage.Tooltip") private MaskingStorage storage = MaskingStorage.NONE; + @HopMetadataProperty + @GuiWidgetElement( + id = WIDGET_TRIM_KEY, + order = "0910", + type = GuiElementType.CHECKBOX, + parentId = GUI_WIDGETS_PARENT_ID, + groupType = GuiWidgetGroupType.BOXES, + group = GROUP_STORAGE, + groupOrder = "30", + label = "i18n::MaskingPattern.TrimKey.Label", + toolTip = "i18n::MaskingPattern.TrimKey.Tooltip") + private boolean trimKey; + + @HopMetadataProperty + @GuiWidgetElement( + id = WIDGET_IGNORE_CASE, + order = "0920", + type = GuiElementType.CHECKBOX, + parentId = GUI_WIDGETS_PARENT_ID, + groupType = GuiWidgetGroupType.BOXES, + group = GROUP_STORAGE, + groupOrder = "30", + label = "i18n::MaskingPattern.IgnoreCase.Label", + toolTip = "i18n::MaskingPattern.IgnoreCase.Tooltip") + private boolean ignoreCase; + @HopMetadataProperty(hopMetadataPropertyType = HopMetadataPropertyType.RDBMS_CONNECTION) @GuiWidgetElement( id = WIDGET_CONNECTION, @@ -215,6 +244,20 @@ public class MaskingPattern extends HopMetadataBase implements IHopMetadata { toolTip = "i18n::MaskingPattern.Table.Tooltip") private String tableName = ""; + @HopMetadataProperty(password = true) + @GuiWidgetElement( + id = WIDGET_HASH_SECRET, + order = "1300", + type = GuiElementType.TEXT, + password = true, + parentId = GUI_WIDGETS_PARENT_ID, + groupType = GuiWidgetGroupType.BOXES, + group = GROUP_STORAGE, + groupOrder = "30", + label = "i18n::MaskingPattern.HashSecret.Label", + toolTip = "i18n::MaskingPattern.HashSecret.Tooltip") + private String hashSecret = ""; + public MaskingPattern() {} /** Suggestions for the classification combo. A project can type its own term. */ diff --git a/plugins/transforms/maskfields/src/main/java/org/apache/hop/pipeline/transforms/maskfields/MaskingPatternEditor.java b/plugins/transforms/maskfields/src/main/java/org/apache/hop/pipeline/transforms/maskfields/MaskingPatternEditor.java index bbe29209a43..19d477614ce 100644 --- a/plugins/transforms/maskfields/src/main/java/org/apache/hop/pipeline/transforms/maskfields/MaskingPatternEditor.java +++ b/plugins/transforms/maskfields/src/main/java/org/apache/hop/pipeline/transforms/maskfields/MaskingPatternEditor.java @@ -152,13 +152,16 @@ private void updateVisibility() { } if (removal) { hidden.add(MaskingPattern.WIDGET_STORAGE); + } + if (removal || storage == MaskingStorage.NONE) { + hidden.add(MaskingPattern.WIDGET_TRIM_KEY); + hidden.add(MaskingPattern.WIDGET_IGNORE_CASE); + } + if (removal || storage != MaskingStorage.DATABASE) { hidden.add(MaskingPattern.WIDGET_CONNECTION); hidden.add(MaskingPattern.WIDGET_SCHEMA); hidden.add(MaskingPattern.WIDGET_TABLE); - } else if (storage != MaskingStorage.DATABASE) { - hidden.add(MaskingPattern.WIDGET_CONNECTION); - hidden.add(MaskingPattern.WIDGET_SCHEMA); - hidden.add(MaskingPattern.WIDGET_TABLE); + hidden.add(MaskingPattern.WIDGET_HASH_SECRET); } widgets.setWidgetsHidden(getMetadata(), hidden); relayoutScrolledContent(); diff --git a/plugins/transforms/maskfields/src/main/java/org/apache/hop/pipeline/transforms/maskfields/MaskingRuntime.java b/plugins/transforms/maskfields/src/main/java/org/apache/hop/pipeline/transforms/maskfields/MaskingRuntime.java index 925b9f8841b..230f3e3c520 100644 --- a/plugins/transforms/maskfields/src/main/java/org/apache/hop/pipeline/transforms/maskfields/MaskingRuntime.java +++ b/plugins/transforms/maskfields/src/main/java/org/apache/hop/pipeline/transforms/maskfields/MaskingRuntime.java @@ -20,6 +20,7 @@ import java.util.Set; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.atomic.AtomicLong; +import org.apache.hop.core.Const; import org.apache.hop.core.database.DatabaseMeta; import org.apache.hop.core.exception.HopException; import org.apache.hop.core.logging.ILoggingObject; @@ -79,7 +80,11 @@ public AtomicLong sequence(String transformName, String fieldName, long start) { return state.sequences.computeIfAbsent(key, name -> new AtomicLong(start)); } - /** One open connection for this database target, shared by every copy in the JVM. */ + /** + * One open connection for this database target, shared by every copy in the JVM. The target is + * the resolved URL and user, so two projects with a connection of the same name that point at + * different databases do not share a store. + */ public DatabaseMaskingStore database( ILoggingObject parent, IVariables variables, @@ -87,7 +92,7 @@ public DatabaseMaskingStore database( String schemaName, String tableName) throws HopException { - String key = databaseKey(databaseMeta, schemaName, tableName); + String key = databaseKey(variables, databaseMeta, schemaName, tableName); while (true) { DatabaseEntry entry = databases.computeIfAbsent(key, name -> new DatabaseEntry()); synchronized (entry) { @@ -150,12 +155,14 @@ public void release() { } } - private static String databaseKey( - DatabaseMeta databaseMeta, String schemaName, String tableName) { - String name = databaseMeta == null ? "" : databaseMeta.getName(); - String schema = schemaName == null ? "" : schemaName; - String table = tableName == null ? "" : tableName; - return name + "\0" + schema + "\0" + table; + static String databaseKey( + IVariables variables, DatabaseMeta databaseMeta, String schemaName, String tableName) + throws HopException { + String url = Const.NVL(databaseMeta.getURL(variables), ""); + String user = Const.NVL(variables.resolve(databaseMeta.getUsername()), ""); + String schema = Const.NVL(schemaName, ""); + String table = Const.NVL(tableName, ""); + return url + "\0" + user + "\0" + schema + "\0" + table; } private static final class ExecutionState { diff --git a/plugins/transforms/maskfields/src/main/java/org/apache/hop/pipeline/transforms/maskfields/store/DatabaseMaskingStore.java b/plugins/transforms/maskfields/src/main/java/org/apache/hop/pipeline/transforms/maskfields/store/DatabaseMaskingStore.java index ca6e3e45f5b..3bd11abe4e3 100644 --- a/plugins/transforms/maskfields/src/main/java/org/apache/hop/pipeline/transforms/maskfields/store/DatabaseMaskingStore.java +++ b/plugins/transforms/maskfields/src/main/java/org/apache/hop/pipeline/transforms/maskfields/store/DatabaseMaskingStore.java @@ -21,6 +21,7 @@ import java.util.Locale; import java.util.Map; import java.util.concurrent.ConcurrentHashMap; +import org.apache.hop.core.Result; import org.apache.hop.core.RowMetaAndData; import org.apache.hop.core.database.Database; import org.apache.hop.core.database.DatabaseMeta; @@ -45,6 +46,12 @@ public class DatabaseMaskingStore implements IMaskingStore { static final String COLUMN_MASKED = "masked_value"; static final String COLUMN_NEXT = "next_value"; + /** Length of the masked value column in a new mapping table. */ + public static final int MASKED_LENGTH = 255; + + /** Attempts of one lookup that loses a race with another process before it gives up. */ + static final int MAX_ATTEMPTS = 5; + private final ILoggingObject parent; private final IVariables variables; private final DatabaseMeta databaseMeta; @@ -90,9 +97,21 @@ public void open() throws HopException { database.setAutoCommit(false); } + @Override + public String findOrCreate(String patternName, String sourceKey, MaskAllocator allocator) + throws HopException { + return findOrCreate(patternName, sourceKey, null, allocator); + } + + /** + * Finds or stores the replacement in one transaction. When another process stores the same source + * key, the next sequence row or the same replacement at the same moment, the constraint violation + * rolls the attempt back and the next attempt sees what that process committed. + */ @Override public synchronized String findOrCreate( - String patternName, String sourceKey, MaskAllocator allocator) throws HopException { + String patternName, String sourceKey, KeySupplier legacyKey, MaskAllocator allocator) + throws HopException { Map patternCache = cache.computeIfAbsent(patternName, k -> new ConcurrentHashMap<>()); String cached = patternCache.get(sourceKey); @@ -100,54 +119,81 @@ public synchronized String findOrCreate( return cached; } ensureOpen(); - try { - String existing = lookup(patternName, sourceKey); - if (existing != null) { + for (int attempt = 1; ; attempt++) { + try { + String masked = lookup(patternName, sourceKey); + String oldKey = masked == null && legacyKey != null ? legacyKey.get() : null; + if (oldKey != null && !oldKey.equals(sourceKey)) { + masked = lookup(patternName, oldKey); + if (masked != null) { + rekey(patternName, oldKey, sourceKey); + } + } + if (masked == null) { + masked = allocator.allocate(this); + insertMap(patternName, sourceKey, masked); + } database.commit(); - patternCache.put(sourceKey, existing); - return existing; - } - String created = allocator.allocate(this); - insertMap(patternName, sourceKey, created); - database.commit(); - patternCache.put(sourceKey, created); - return created; - } catch (HopException e) { - rollbackQuietly(); - if (isConstraintViolation(e)) { - String winner = lookup(patternName, sourceKey); - if (winner != null) { - database.commit(); - patternCache.put(sourceKey, winner); - return winner; + patternCache.put(sourceKey, masked); + return masked; + } catch (HopException e) { + rollbackQuietly(); + if (!isConstraintViolation(e)) { + throw e; + } + if (attempt >= MAX_ATTEMPTS) { + // The database's message names the conflicting key, which can be the original value. + // Leave the cause out so that value does not reach the log. + throw new HopException( + "Unable to store a replacement for pattern '" + + patternName + + "' after " + + MAX_ATTEMPTS + + " attempts: other processes kept storing conflicting rows in " + + mapTable); } } - throw e; } } + /** + * Next sequence value. The update locks the pattern's row until the caller commits, so a second + * process waits and then continues from the committed value. + */ @Override public synchronized long allocateSequence(String patternName, long start) throws HopException { ensureOpen(); - Long current = queryLong(selectNextSql(), oneString(), new Object[] {patternName}); - if (current == null) { - exec( - "INSERT INTO " - + sequenceTable - + " (" - + quotedPattern - + ", " - + quotedNext - + ") VALUES (?, ?)", - stringAndLong(), - new Object[] {patternName, start + 1}); - return start; + Result result = + database.execStatement( + "UPDATE " + + sequenceTable + + " SET " + + quotedNext + + " = " + + quotedNext + + " + 1 WHERE " + + quotedPattern + + " = ?", + oneString(), + new Object[] {patternName}); + if (result.getNrLinesUpdated() > 0) { + Long next = queryLong(selectNextSql(), oneString(), new Object[] {patternName}); + if (next == null) { + throw new HopException("The sequence row for pattern '" + patternName + "' disappeared"); + } + return next - 1; } exec( - "UPDATE " + sequenceTable + " SET " + quotedNext + " = ? WHERE " + quotedPattern + " = ?", - longAndString(), - new Object[] {current + 1, patternName}); - return current; + "INSERT INTO " + + sequenceTable + + " (" + + quotedPattern + + ", " + + quotedNext + + ") VALUES (?, ?)", + stringAndLong(), + new Object[] {patternName, start + 1}); + return start; } @Override @@ -175,7 +221,9 @@ private void createMapTable() throws HopException { return; } // Primary-key columns have to be NOT NULL. The generic CREATE TABLE leaves them nullable, and - // H2 rejects a primary key on a nullable column. + // H2 rejects a primary key on a nullable column. The unique key stops two source values from + // sharing a replacement, which keeps the masked value short enough to index on every database. + // Tables created by earlier versions keep their layout. database.execStatement( "CREATE TABLE " + mapTable @@ -184,11 +232,15 @@ private void createMapTable() throws HopException { + ", " + columnDefinition(stringColumn(COLUMN_SOURCE, 255)) + ", " - + columnDefinition(stringColumn(COLUMN_MASKED, 2000)) + + columnDefinition(stringColumn(COLUMN_MASKED, MASKED_LENGTH)) + ", PRIMARY KEY (" + quotedPattern + ", " + quotedSource + + "), UNIQUE (" + + quotedPattern + + ", " + + quotedMasked + "))"); } @@ -249,6 +301,21 @@ private void insertMap(String patternName, String sourceKey, String masked) thro new Object[] {patternName, sourceKey, masked}); } + private void rekey(String patternName, String oldKey, String newKey) throws HopException { + exec( + "UPDATE " + + mapTable + + " SET " + + quotedSource + + " = ? WHERE " + + quotedPattern + + " = ? AND " + + quotedSource + + " = ?", + rekeyParameters(), + new Object[] {newKey, patternName, oldKey}); + } + private String selectNextSql() { return "SELECT " + quotedNext + " FROM " + sequenceTable + " WHERE " + quotedPattern + " = ?"; } @@ -342,16 +409,17 @@ private static RowMeta threeStrings() { return meta; } - private static IRowMeta stringAndLong() { - RowMeta meta = oneString(); - meta.addValueMeta(new ValueMetaInteger(COLUMN_NEXT)); + private static RowMeta rekeyParameters() { + RowMeta meta = new RowMeta(); + meta.addValueMeta(new ValueMetaString(COLUMN_SOURCE)); + meta.addValueMeta(new ValueMetaString(COLUMN_PATTERN)); + meta.addValueMeta(new ValueMetaString("old_" + COLUMN_SOURCE)); return meta; } - private static IRowMeta longAndString() { - RowMeta meta = new RowMeta(); + private static IRowMeta stringAndLong() { + RowMeta meta = oneString(); meta.addValueMeta(new ValueMetaInteger(COLUMN_NEXT)); - meta.addValueMeta(new ValueMetaString(COLUMN_PATTERN)); return meta; } } diff --git a/plugins/transforms/maskfields/src/main/java/org/apache/hop/pipeline/transforms/maskfields/store/IMaskingStore.java b/plugins/transforms/maskfields/src/main/java/org/apache/hop/pipeline/transforms/maskfields/store/IMaskingStore.java index cb0380c6efd..78685d56e9f 100644 --- a/plugins/transforms/maskfields/src/main/java/org/apache/hop/pipeline/transforms/maskfields/store/IMaskingStore.java +++ b/plugins/transforms/maskfields/src/main/java/org/apache/hop/pipeline/transforms/maskfields/store/IMaskingStore.java @@ -31,6 +31,18 @@ public interface IMaskingStore extends AutoCloseable { String findOrCreate(String patternName, String sourceKey, MaskAllocator allocator) throws HopException; + /** + * Like {@link #findOrCreate(String, String, MaskAllocator)}, but also finds a mapping stored + * under the key an earlier version wrote, and moves it to {@code sourceKey}. The store only asks + * for that key when {@code sourceKey} is not found. A store that never kept mappings across + * versions ignores it. + */ + default String findOrCreate( + String patternName, String sourceKey, KeySupplier legacyKey, MaskAllocator allocator) + throws HopException { + return findOrCreate(patternName, sourceKey, allocator); + } + /** Next sequence value for the pattern, starting at {@code start} the first time. */ long allocateSequence(String patternName, long start) throws HopException; @@ -40,6 +52,12 @@ String findOrCreate(String patternName, String sourceKey, MaskAllocator allocato @Override void close(); + /** Supplies a key only when the store needs it. */ + @FunctionalInterface + interface KeySupplier { + String get() throws HopException; + } + /** Builds the replacement string for a new source key. */ @FunctionalInterface interface MaskAllocator { diff --git a/plugins/transforms/maskfields/src/main/resources/org/apache/hop/pipeline/transforms/maskfields/messages/messages_en_US.properties b/plugins/transforms/maskfields/src/main/resources/org/apache/hop/pipeline/transforms/maskfields/messages/messages_en_US.properties index c2292826e69..7291516c693 100644 --- a/plugins/transforms/maskfields/src/main/resources/org/apache/hop/pipeline/transforms/maskfields/messages/messages_en_US.properties +++ b/plugins/transforms/maskfields/src/main/resources/org/apache/hop/pipeline/transforms/maskfields/messages/messages_en_US.properties @@ -46,10 +46,13 @@ MaskFields.Check.Incompatible=Field ''{0}'' cannot use masking pattern ''{1}'': MaskFields.Check.NoConnection=Masking pattern ''{0}'' stores mappings in a database and has no connection. MaskFields.Check.MissingConnection=Database connection ''{0}'' for masking pattern ''{1}'' was not found. MaskFields.Check.NoTable=Masking pattern ''{0}'' stores mappings in a database and has no table name. +MaskFields.Check.MaskedTooLong=Masking pattern ''{0}'' can write values of up to {1} characters. A mapping table created by this version holds {2}. +MaskFields.Check.PlainTextKeys=Masking pattern ''{0}'' has no hash secret, so its mapping table stores the original values in plain text. MaskFields.Error.FieldMissing=Field ''{0}'' is no longer in the input row. MaskFields.Error.Convert=Unable to write the masked value into field ''{0}'' with pattern ''{1}''. MaskFields.Error.BadSequence=Sequence start ''{0}'' on masking pattern ''{1}'' is not an integer. +MaskFields.Error.HashSecretUnresolved=The hash secret of masking pattern ''{0}'' is empty or holds a variable that is not set. MaskFields.Error.OpenStore=Unable to open the mapping table for masking pattern ''{0}''. MaskingRules.SetEmpty=Set to blank only applies to a string field. @@ -89,10 +92,16 @@ MaskingPattern.Suffix.Tooltip=Text written after the token. MaskingPattern.SequenceStart.Label=Sequence start MaskingPattern.SequenceStart.Tooltip=First integer to use. The default is 1. MaskingPattern.Storage.Label=Remember values -MaskingPattern.Storage.Tooltip=NONE assigns a new value every row. MEMORY keeps the mapping for this run. DATABASE keeps it in a table across runs. The table stores the original value. +MaskingPattern.Storage.Tooltip=NONE assigns a new value every row. MEMORY keeps the mapping for this run. DATABASE keeps it in a table across runs. +MaskingPattern.TrimKey.Label=Ignore surrounding spaces +MaskingPattern.TrimKey.Tooltip=Values that only differ in leading or trailing spaces get the same replacement. A value of only spaces is left unchanged. +MaskingPattern.IgnoreCase.Label=Ignore case +MaskingPattern.IgnoreCase.Tooltip=Values that only differ in upper and lower case get the same replacement. +MaskingPattern.HashSecret.Label=Hash secret +MaskingPattern.HashSecret.Tooltip=The mapping table stores an HMAC-SHA256 of each value made with this secret, instead of the value itself. Use a variable such as '${MASKING_SECRET}' and keep the secret out of the project. Changing the secret starts a new mapping. Without a secret the table stores the original values. MaskingPattern.Connection.Label=Connection MaskingPattern.Connection.Tooltip=Database that holds the mapping table. MaskingPattern.Schema.Label=Schema MaskingPattern.Schema.Tooltip=Schema of the mapping table. Leave this empty to use the connection default. MaskingPattern.Table.Label=Table -MaskingPattern.Table.Tooltip=Mapping table. A second table with the suffix _seq stores the next sequence number. Both are created when they are missing. +MaskingPattern.Table.Tooltip=Mapping table. A second table with the suffix _seq stores the next sequence number. Both are created when they are missing. A new mapping table holds replacements of up to 255 characters. diff --git a/plugins/transforms/maskfields/src/test/java/org/apache/hop/pipeline/transforms/maskfields/MaskFieldsMetaTest.java b/plugins/transforms/maskfields/src/test/java/org/apache/hop/pipeline/transforms/maskfields/MaskFieldsMetaTest.java index 8cd58245a98..130046b72e5 100644 --- a/plugins/transforms/maskfields/src/test/java/org/apache/hop/pipeline/transforms/maskfields/MaskFieldsMetaTest.java +++ b/plugins/transforms/maskfields/src/test/java/org/apache/hop/pipeline/transforms/maskfields/MaskFieldsMetaTest.java @@ -18,6 +18,7 @@ package org.apache.hop.pipeline.transforms.maskfields; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertNull; import static org.junit.jupiter.api.Assertions.assertTrue; @@ -86,6 +87,55 @@ void rejectsPatternsThatDoNotFitTheField() throws Exception { assertTrue(messages(remarks).stream().anyMatch(text -> text.contains("Missing"))); } + @Test + void warnsAboutPlainTextKeysAndLongReplacements() throws Exception { + MemoryMetadataProvider provider = new MemoryMetadataProvider(); + MaskingPattern plain = synthetic("Plain", MaskingToken.SEQUENCE); + plain.setStorage(MaskingStorage.DATABASE); + plain.setConnection("db"); + plain.setTableName("mask_map"); + save(provider, plain); + MaskingPattern hashed = synthetic("Hashed", MaskingToken.UUID); + hashed.setStorage(MaskingStorage.DATABASE); + hashed.setConnection("db"); + hashed.setTableName("mask_map"); + hashed.setHashSecret("${SECRET}"); + hashed.setPrefix("${PREFIX}"); + save(provider, hashed); + + MaskFieldsMeta meta = new MaskFieldsMeta(); + meta.getFields().add(new MaskField("name", "Plain")); + meta.getFields().add(new MaskField("city", "Hashed")); + RowMeta prev = new RowMeta(); + prev.addValueMeta(new ValueMetaString("name")); + prev.addValueMeta(new ValueMetaString("city")); + Variables variables = new Variables(); + variables.setVariable("PREFIX", "x".repeat(250)); + + List remarks = new ArrayList<>(); + TransformMeta transform = new TransformMeta(); + transform.setName("Mask"); + meta.check( + remarks, + null, + transform, + prev, + new String[] {"in"}, + new String[0], + null, + variables, + provider); + + List warnings = + remarks.stream() + .filter(remark -> remark.getType() == ICheckResult.TYPE_RESULT_WARNING) + .map(ICheckResult::getText) + .toList(); + assertEquals(2, warnings.size(), warnings.toString()); + assertTrue(warnings.get(0).contains("Plain") && warnings.get(0).contains("plain text")); + assertTrue(warnings.get(1).contains("Hashed") && warnings.get(1).contains("286")); + } + @Test void roundTripsTheTransformAndThePattern() throws Exception { MaskFieldsMeta meta = new MaskFieldsMeta(); @@ -101,6 +151,9 @@ void roundTripsTheTransformAndThePattern() throws Exception { pattern.setPrefix("first-name-"); pattern.setPiiClassification("Direct identifier"); pattern.setStorage(MaskingStorage.MEMORY); + pattern.setTrimKey(true); + pattern.setIgnoreCase(true); + pattern.setHashSecret("s3cret"); MemoryMetadataProvider provider = new MemoryMetadataProvider(); JsonMetadataParser parser = new JsonMetadataParser<>(MaskingPattern.class, provider); @@ -113,6 +166,10 @@ void roundTripsTheTransformAndThePattern() throws Exception { assertEquals("Direct identifier", loaded.getPiiClassification()); assertEquals(MaskingStorage.MEMORY, loaded.getStorage()); assertEquals(MaskingValueSource.SYNTHETIC, loaded.getValueSource()); + assertTrue(loaded.isTrimKey()); + assertTrue(loaded.isIgnoreCase()); + assertEquals("s3cret", loaded.getHashSecret()); + assertFalse(json.toJSONString().contains("s3cret")); assertNull(MaskingRules.incompatibility(new ValueMetaString("name"), loaded)); } } diff --git a/plugins/transforms/maskfields/src/test/java/org/apache/hop/pipeline/transforms/maskfields/MaskingEngineTest.java b/plugins/transforms/maskfields/src/test/java/org/apache/hop/pipeline/transforms/maskfields/MaskingEngineTest.java index bc4a51ac094..1a670ca33ad 100644 --- a/plugins/transforms/maskfields/src/test/java/org/apache/hop/pipeline/transforms/maskfields/MaskingEngineTest.java +++ b/plugins/transforms/maskfields/src/test/java/org/apache/hop/pipeline/transforms/maskfields/MaskingEngineTest.java @@ -17,15 +17,19 @@ package org.apache.hop.pipeline.transforms.maskfields; +import static org.junit.jupiter.api.Assertions.assertArrayEquals; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; import java.util.List; import java.util.UUID; +import org.apache.hop.core.exception.HopException; import org.apache.hop.core.row.RowMeta; import org.apache.hop.core.row.value.ValueMetaString; import org.apache.hop.pipeline.transforms.maskfields.MaskingEngine.Binding; +import org.apache.hop.pipeline.transforms.maskfields.store.IMaskingStore; import org.apache.hop.pipeline.transforms.maskfields.store.MemoryMaskingStore; import org.junit.jupiter.api.Test; @@ -94,6 +98,45 @@ void setNullAndSetEmpty() throws Exception { assertEquals("", masked[1]); } + @Test + void aFailingFieldClearsItselfAndTheFieldsAfterIt() throws Exception { + MaskingPattern first = synthetic("First name", MaskingStorage.NONE, "fn-", "", "1"); + MaskingPattern broken = synthetic("Broken", MaskingStorage.MEMORY, "x-", "", "1"); + IMaskingStore failing = + new MemoryMaskingStore() { + @Override + public synchronized String findOrCreate( + String patternName, String sourceKey, MaskAllocator allocator) throws HopException { + throw new HopException("mapping table is gone"); + } + }; + MaskingEngine engine = + engine( + binding("first", first, null), + new Binding("last", broken, "x-", "", 1, failing), + binding("city", first, null)); + RowMeta rowMeta = stringRow("first", "last", "city", "untouched"); + Object[] row = {"Matt", "Casters", "Gent", "kept"}; + + assertThrows(HopException.class, () -> engine.apply(rowMeta, row)); + assertArrayEquals(new Object[] {"fn-1", null, null, "kept"}, row); + } + + @Test + void aRememberedValueIgnoresCaseAndSpacesWhenAsked() throws Exception { + MaskingPattern pattern = synthetic("First name", MaskingStorage.MEMORY, "fn-", "", "1"); + MemoryMaskingStore store = new MemoryMaskingStore(); + long start = 1L; + MaskingEngine engine = + engine( + new Binding( + "name", pattern, "fn-", "", start, store, null, new MaskingKey(true, true, null))); + RowMeta rowMeta = stringRow("name"); + assertEquals("fn-1", apply(engine, rowMeta, "Matt")[0]); + assertEquals("fn-1", apply(engine, rowMeta, " MATT ")[0]); + assertEquals(" ", apply(engine, rowMeta, " ")[0]); + } + private static MaskingPattern synthetic( String name, MaskingStorage storage, String prefix, String suffix, String start) { MaskingPattern pattern = new MaskingPattern(); @@ -107,7 +150,7 @@ private static MaskingPattern synthetic( return pattern; } - private static Binding binding(String field, MaskingPattern pattern, MemoryMaskingStore store) { + private static Binding binding(String field, MaskingPattern pattern, IMaskingStore store) { long start = 1L; if (pattern.getSequenceStart() != null && !pattern.getSequenceStart().isBlank()) { start = Long.parseLong(pattern.getSequenceStart().trim()); diff --git a/plugins/transforms/maskfields/src/test/java/org/apache/hop/pipeline/transforms/maskfields/MaskingKeyTest.java b/plugins/transforms/maskfields/src/test/java/org/apache/hop/pipeline/transforms/maskfields/MaskingKeyTest.java new file mode 100644 index 00000000000..10748449361 --- /dev/null +++ b/plugins/transforms/maskfields/src/test/java/org/apache/hop/pipeline/transforms/maskfields/MaskingKeyTest.java @@ -0,0 +1,105 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.hop.pipeline.transforms.maskfields; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNotEquals; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import java.math.BigDecimal; +import org.apache.hop.core.exception.HopException; +import org.apache.hop.core.row.value.ValueMetaBigNumber; +import org.apache.hop.core.row.value.ValueMetaInteger; +import org.apache.hop.core.row.value.ValueMetaNumber; +import org.apache.hop.core.row.value.ValueMetaString; +import org.apache.hop.core.variables.Variables; +import org.junit.jupiter.api.Test; + +class MaskingKeyTest { + + @Test + void aNumberKeyDoesNotDependOnTheConversionMask() throws Exception { + MaskingKey key = MaskingKey.plain(); + ValueMetaNumber plain = new ValueMetaNumber("amount"); + ValueMetaNumber formatted = new ValueMetaNumber("amount"); + formatted.setConversionMask("#,##0.00"); + assertEquals("1234.5", key.normalize(plain, 1234.5)); + assertEquals("1234.5", key.normalize(formatted, 1234.5)); + + ValueMetaInteger integer = new ValueMetaInteger("id"); + integer.setConversionMask("000000"); + assertEquals("42", key.normalize(integer, 42L)); + assertEquals("000042", MaskingKey.legacyKey(integer, 42L)); + + assertEquals("10", key.normalize(new ValueMetaBigNumber("big"), new BigDecimal("10.000"))); + } + + @Test + void trimAndIgnoreCaseAreOptional() throws Exception { + ValueMetaString name = new ValueMetaString("name"); + assertEquals(" Matt ", MaskingKey.plain().normalize(name, " Matt ")); + assertEquals("matt", new MaskingKey(true, true, null).normalize(name, " Matt ")); + assertEquals("", new MaskingKey(true, false, null).normalize(name, " ")); + } + + @Test + void aHashSecretThatDoesNotResolveIsAnError() { + Variables variables = new Variables(); + variables.setVariable("EMPTY_SECRET", ""); + for (String secret : new String[] {"${NOT_SET}", "${EMPTY_SECRET}", "%%NOT_SET%%"}) { + MaskingPattern pattern = databasePattern(secret); + HopException e = + assertThrows(HopException.class, () -> MaskingKey.forPattern(pattern, variables)); + assertTrue(e.getMessage().contains("Names"), e.getMessage()); + } + } + + @Test + void aResolvedHashSecretHashesDatabaseKeysOnly() throws Exception { + Variables variables = new Variables(); + variables.setVariable("SECRET", "s3cret"); + String expected = new MaskingKey(false, false, "s3cret").storeKey("Matt"); + assertEquals( + expected, MaskingKey.forPattern(databasePattern("${SECRET}"), variables).storeKey("Matt")); + + MaskingPattern memory = databasePattern("${NOT_SET}"); + memory.setStorage(MaskingStorage.MEMORY); + assertEquals("Matt", MaskingKey.forPattern(memory, variables).storeKey("Matt")); + assertEquals("Matt", MaskingKey.forPattern(databasePattern(""), variables).storeKey("Matt")); + } + + private static MaskingPattern databasePattern(String secret) { + MaskingPattern pattern = new MaskingPattern(); + pattern.setName("Names"); + pattern.setStorage(MaskingStorage.DATABASE); + pattern.setHashSecret(secret); + return pattern; + } + + @Test + void aSecretHashesTheStoredKey() throws Exception { + MaskingKey key = new MaskingKey(false, false, "s3cret"); + String hashed = key.storeKey("Matt"); + assertTrue(hashed.startsWith(MaskingKey.HMAC_PREFIX)); + assertEquals(MaskingKey.HMAC_PREFIX.length() + 64, hashed.length()); + assertEquals(hashed, new MaskingKey(false, false, "s3cret").storeKey("Matt")); + assertNotEquals(hashed, new MaskingKey(false, false, "other").storeKey("Matt")); + assertEquals("Matt", MaskingKey.plain().storeKey("Matt")); + } +} diff --git a/plugins/transforms/maskfields/src/test/java/org/apache/hop/pipeline/transforms/maskfields/store/DatabaseMaskingStoreTest.java b/plugins/transforms/maskfields/src/test/java/org/apache/hop/pipeline/transforms/maskfields/store/DatabaseMaskingStoreTest.java index 5695dd17e1f..e7850fb0adc 100644 --- a/plugins/transforms/maskfields/src/test/java/org/apache/hop/pipeline/transforms/maskfields/store/DatabaseMaskingStoreTest.java +++ b/plugins/transforms/maskfields/src/test/java/org/apache/hop/pipeline/transforms/maskfields/store/DatabaseMaskingStoreTest.java @@ -18,13 +18,29 @@ package org.apache.hop.pipeline.transforms.maskfields.store; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertNotSame; +import static org.junit.jupiter.api.Assertions.assertNull; import static org.junit.jupiter.api.Assertions.assertSame; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; +import java.util.ArrayList; +import java.util.HashSet; +import java.util.List; +import java.util.Set; import java.util.UUID; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.Executors; +import java.util.concurrent.Future; +import java.util.concurrent.TimeUnit; import org.apache.hop.core.HopEnvironment; +import org.apache.hop.core.database.Database; import org.apache.hop.core.database.DatabaseMeta; +import org.apache.hop.core.exception.HopException; import org.apache.hop.core.logging.ILoggingObject; import org.apache.hop.core.variables.Variables; +import org.apache.hop.pipeline.transforms.maskfields.MaskingKey; import org.apache.hop.pipeline.transforms.maskfields.MaskingRuntime; import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.Test; @@ -124,8 +140,159 @@ void runtimeSharesOneDatabaseStoreAcrossCopies() throws Exception { } } + @Test + void twoProcessesNeverHandOutTheSameValue() throws Exception { + DatabaseMeta databaseMeta = h2(); + Variables variables = new Variables(); + // Two stores on one table stand in for two Hop processes: no shared lock, two connections. + DatabaseMaskingStore first = + new DatabaseMaskingStore((ILoggingObject) null, variables, databaseMeta, null, "mask_map"); + DatabaseMaskingStore second = + new DatabaseMaskingStore((ILoggingObject) null, variables, databaseMeta, null, "mask_map"); + first.open(); + second.open(); + int perStore = 200; + // The first value creates the sequence row. Both processes then race on updating it. + Set masked = new HashSet<>(List.of(allocate(first, "seed"))); + ExecutorService executor = Executors.newFixedThreadPool(2); + try { + Future> a = executor.submit(() -> allocateAll(first, "a", perStore)); + Future> b = executor.submit(() -> allocateAll(second, "b", perStore)); + masked.addAll(a.get(60, TimeUnit.SECONDS)); + masked.addAll(b.get(60, TimeUnit.SECONDS)); + assertEquals(2 * perStore + 1, masked.size()); + } finally { + executor.shutdownNow(); + first.close(); + second.close(); + } + } + + @Test + void aHashSecretKeepsOriginalValuesOutOfTheTable() throws Exception { + DatabaseMeta databaseMeta = h2(); + Variables variables = new Variables(); + MaskingKey key = new MaskingKey(false, false, "s3cret"); + DatabaseMaskingStore store = + new DatabaseMaskingStore((ILoggingObject) null, variables, databaseMeta, null, "mask_map"); + store.open(); + try { + String hashed = key.storeKey("Matt"); + assertTrue(hashed.startsWith("hmac-sha256:")); + assertEquals("first-name-1", allocate(store, hashed, "Matt")); + assertEquals("first-name-1", allocate(store, hashed, "Matt")); + assertEquals(List.of(hashed), sourceKeys(databaseMeta, variables)); + } finally { + store.close(); + } + } + + @Test + void aPlainTextRowMovesToItsHashedKey() throws Exception { + DatabaseMeta databaseMeta = h2(); + Variables variables = new Variables(); + DatabaseMaskingStore store = + new DatabaseMaskingStore((ILoggingObject) null, variables, databaseMeta, null, "mask_map"); + store.open(); + try { + // A row written before the pattern had a hash secret. + assertEquals("first-name-1", allocate(store, "Matt")); + } finally { + store.close(); + } + + String hashed = new MaskingKey(false, false, "s3cret").storeKey("Matt"); + DatabaseMaskingStore later = + new DatabaseMaskingStore((ILoggingObject) null, variables, databaseMeta, null, "mask_map"); + later.open(); + try { + assertEquals("first-name-1", allocate(later, hashed, "Matt")); + assertEquals("first-name-2", allocate(later, hashed + "x", "Ann")); + } finally { + later.close(); + } + List keys = sourceKeys(databaseMeta, variables); + assertTrue(keys.contains(hashed)); + assertFalse(keys.contains("Matt")); + } + + @Test + void connectionsWithTheSameNameOnDifferentDatabasesGetTheirOwnStore() throws Exception { + Variables variables = new Variables(); + DatabaseMeta one = h2(); + DatabaseMeta two = h2(); + two.setName(one.getName()); + MaskingRuntime.Lease lease = MaskingRuntime.getInstance().acquire("db-" + UUID.randomUUID()); + try { + DatabaseMaskingStore first = lease.database(null, variables, one, null, "mask_map"); + DatabaseMaskingStore second = lease.database(null, variables, two, null, "mask_map"); + assertNotSame(first, second); + assertEquals("first-name-1", allocate(first, "Matt")); + assertEquals("first-name-1", allocate(second, "Ann")); + } finally { + lease.release(); + } + } + + @Test + void anExhaustedRetryKeepsTheSourceKeyOutOfTheError() throws Exception { + DatabaseMaskingStore store = + new DatabaseMaskingStore((ILoggingObject) null, new Variables(), h2(), null, "mask_map"); + store.open(); + try { + // Every new key gets the same replacement, so the unique key on it rejects every attempt. + assertEquals("same", store.findOrCreate("First name", "Matt", current -> "same")); + HopException e = + assertThrows( + HopException.class, () -> store.findOrCreate("First name", "Ann", current -> "same")); + assertNull(e.getCause()); + assertFalse(e.getMessage().contains("Ann"), e.getMessage()); + assertTrue(e.getMessage().contains("First name"), e.getMessage()); + } finally { + store.close(); + } + } + + private static List sourceKeys(DatabaseMeta databaseMeta, Variables variables) + throws Exception { + Database database = new Database((ILoggingObject) null, variables, databaseMeta); + database.connect(); + try { + List keys = new ArrayList<>(); + for (Object[] row : database.getRows("SELECT source_key FROM mask_map", 1000)) { + keys.add((String) row[0]); + } + return keys; + } finally { + database.disconnect(); + } + } + + private static List allocateAll(DatabaseMaskingStore store, String prefix, int count) + throws Exception { + List masked = new ArrayList<>(); + for (int i = 0; i < count; i++) { + masked.add(allocate(store, prefix + i)); + } + return masked; + } + + private static DatabaseMeta h2() { + String id = UUID.randomUUID().toString().replace("-", ""); + return new DatabaseMeta( + "mask-" + id, "H2", "Native", "", "mem:mask" + id + ";DB_CLOSE_DELAY=-1", "", "sa", ""); + } + private static String allocate(DatabaseMaskingStore store, String source) throws Exception { + return allocate(store, source, null); + } + + private static String allocate(DatabaseMaskingStore store, String source, String legacy) + throws Exception { return store.findOrCreate( - "First name", source, current -> "first-name-" + current.allocateSequence("First name", 1)); + "First name", + source, + legacy == null ? null : () -> legacy, + current -> "first-name-" + current.allocateSequence("First name", 1)); } } From 076baa23207def45c05516b1832b1d9c1c0adcb1 Mon Sep 17 00:00:00 2001 From: Bart Maertens Date: Fri, 9 Oct 2026 16:00:53 +0200 Subject: [PATCH 2/3] Address review: resolve manual URLs in the mapping store key, reject undecryptable hash secrets, remove leftover legacy rows (#8791) --- .../transforms/maskfields/MaskingKey.java | 11 ++-- .../transforms/maskfields/MaskingRuntime.java | 32 +++++++++- .../store/DatabaseMaskingStore.java | 43 ++++++++++--- .../maskfields/store/IMaskingStore.java | 6 +- .../messages/messages_en_US.properties | 2 +- .../transforms/maskfields/MaskingKeyTest.java | 6 +- .../maskfields/MaskingRuntimeTest.java | 21 +++++++ .../store/DatabaseMaskingStoreTest.java | 62 +++++++++++++++++++ 8 files changed, 165 insertions(+), 18 deletions(-) diff --git a/plugins/transforms/maskfields/src/main/java/org/apache/hop/pipeline/transforms/maskfields/MaskingKey.java b/plugins/transforms/maskfields/src/main/java/org/apache/hop/pipeline/transforms/maskfields/MaskingKey.java index 8f31a0aca6e..7883cfa08f2 100644 --- a/plugins/transforms/maskfields/src/main/java/org/apache/hop/pipeline/transforms/maskfields/MaskingKey.java +++ b/plugins/transforms/maskfields/src/main/java/org/apache/hop/pipeline/transforms/maskfields/MaskingKey.java @@ -67,8 +67,8 @@ public MaskingKey(boolean trim, boolean ignoreCase, String secret) throws HopExc /** * The key for a pattern. Only a database pattern hashes its keys. A hash secret that resolves to - * nothing, or still holds a variable, is an error: hashing with the literal text or falling back - * to plain text would both go unnoticed. + * nothing, still holds a variable or does not decrypt is an error: hashing with the literal text + * or falling back to plain text would both go unnoticed. */ public static MaskingKey forPattern(MaskingPattern pattern, IVariables variables) throws HopException { @@ -76,12 +76,15 @@ public static MaskingKey forPattern(MaskingPattern pattern, IVariables variables if (pattern.getStorage() == MaskingStorage.DATABASE && StringUtils.isNotEmpty(pattern.getHashSecret())) { String resolved = variables.resolve(pattern.getHashSecret()); - if (StringUtils.isEmpty(resolved) || StringUtil.containsVariableToken(resolved)) { + if (!StringUtils.isEmpty(resolved) && !StringUtil.containsVariableToken(resolved)) { + // An encrypted value that is not a valid ciphertext decrypts to an empty string. + secret = Encr.decryptPasswordOptionallyEncrypted(resolved); + } + if (StringUtils.isEmpty(secret)) { throw new HopException( BaseMessages.getString( PKG, "MaskFields.Error.HashSecretUnresolved", pattern.getName())); } - secret = Encr.decryptPasswordOptionallyEncrypted(resolved); } return new MaskingKey(pattern.isTrimKey(), pattern.isIgnoreCase(), secret); } diff --git a/plugins/transforms/maskfields/src/main/java/org/apache/hop/pipeline/transforms/maskfields/MaskingRuntime.java b/plugins/transforms/maskfields/src/main/java/org/apache/hop/pipeline/transforms/maskfields/MaskingRuntime.java index 230f3e3c520..9ca3838578d 100644 --- a/plugins/transforms/maskfields/src/main/java/org/apache/hop/pipeline/transforms/maskfields/MaskingRuntime.java +++ b/plugins/transforms/maskfields/src/main/java/org/apache/hop/pipeline/transforms/maskfields/MaskingRuntime.java @@ -17,13 +17,16 @@ package org.apache.hop.pipeline.transforms.maskfields; +import java.util.List; import java.util.Set; import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.CopyOnWriteArrayList; import java.util.concurrent.atomic.AtomicLong; import org.apache.hop.core.Const; import org.apache.hop.core.database.DatabaseMeta; import org.apache.hop.core.exception.HopException; import org.apache.hop.core.logging.ILoggingObject; +import org.apache.hop.core.util.StringUtil; import org.apache.hop.core.variables.IVariables; import org.apache.hop.pipeline.transforms.maskfields.store.DatabaseMaskingStore; import org.apache.hop.pipeline.transforms.maskfields.store.MemoryMaskingStore; @@ -60,6 +63,7 @@ public final class Lease { private final String executionId; private final ExecutionState state; private final Set databaseKeys = ConcurrentHashMap.newKeySet(); + private final List ownStores = new CopyOnWriteArrayList<>(); private boolean released; private Lease(String executionId, ExecutionState state) { @@ -83,7 +87,8 @@ public AtomicLong sequence(String transformName, String fieldName, long start) { /** * One open connection for this database target, shared by every copy in the JVM. The target is * the resolved URL and user, so two projects with a connection of the same name that point at - * different databases do not share a store. + * different databases do not share a store. A URL that still holds a variable does not say + * which database it opens, so this copy gets a store of its own. */ public DatabaseMaskingStore database( ILoggingObject parent, @@ -93,6 +98,18 @@ public DatabaseMaskingStore database( String tableName) throws HopException { String key = databaseKey(variables, databaseMeta, schemaName, tableName); + if (key == null) { + DatabaseMaskingStore store = + new DatabaseMaskingStore(parent, variables, databaseMeta, schemaName, tableName); + try { + store.open(); + } catch (HopException e) { + store.close(); + throw e; + } + ownStores.add(store); + return store; + } while (true) { DatabaseEntry entry = databases.computeIfAbsent(key, name -> new DatabaseEntry()); synchronized (entry) { @@ -136,6 +153,9 @@ public void release() { state.memory.close(); } } + for (DatabaseMaskingStore store : ownStores) { + store.close(); + } for (String key : databaseKeys) { DatabaseEntry entry = databases.get(key); if (entry == null) { @@ -155,10 +175,18 @@ public void release() { } } + /** + * @return the URL a connection opens, its user and the table, or null when the URL still holds a + * variable + */ static String databaseKey( IVariables variables, DatabaseMeta databaseMeta, String schemaName, String tableName) throws HopException { - String url = Const.NVL(databaseMeta.getURL(variables), ""); + // A manual URL comes back from getURL() as entered. Database.connect() resolves it. + String url = Const.NVL(variables.resolve(databaseMeta.getURL(variables)), ""); + if (StringUtil.containsVariableToken(url)) { + return null; + } String user = Const.NVL(variables.resolve(databaseMeta.getUsername()), ""); String schema = Const.NVL(schemaName, ""); String table = Const.NVL(tableName, ""); diff --git a/plugins/transforms/maskfields/src/main/java/org/apache/hop/pipeline/transforms/maskfields/store/DatabaseMaskingStore.java b/plugins/transforms/maskfields/src/main/java/org/apache/hop/pipeline/transforms/maskfields/store/DatabaseMaskingStore.java index 3bd11abe4e3..ee4a6c00acf 100644 --- a/plugins/transforms/maskfields/src/main/java/org/apache/hop/pipeline/transforms/maskfields/store/DatabaseMaskingStore.java +++ b/plugins/transforms/maskfields/src/main/java/org/apache/hop/pipeline/transforms/maskfields/store/DatabaseMaskingStore.java @@ -20,6 +20,7 @@ import java.sql.SQLException; import java.util.Locale; import java.util.Map; +import java.util.Set; import java.util.concurrent.ConcurrentHashMap; import org.apache.hop.core.Result; import org.apache.hop.core.RowMetaAndData; @@ -59,6 +60,9 @@ public class DatabaseMaskingStore implements IMaskingStore { private final String tableName; private final Map> cache = new ConcurrentHashMap<>(); + /** Legacy keys per pattern this store already looked up and moved or removed. */ + private final Map> legacyDone = new ConcurrentHashMap<>(); + private Database database; private String mapTable; private String sequenceTable; @@ -107,6 +111,10 @@ public String findOrCreate(String patternName, String sourceKey, MaskAllocator a * Finds or stores the replacement in one transaction. When another process stores the same source * key, the next sequence row or the same replacement at the same moment, the constraint violation * rolls the attempt back and the next attempt sees what that process committed. + * + *

A row under the legacy key moves to {@code sourceKey}. When {@code sourceKey} already has a + * row, for example because a second spelling folds onto the same key, the legacy row is removed + * so the original value does not stay in the table. */ @Override public synchronized String findOrCreate( @@ -114,19 +122,29 @@ public synchronized String findOrCreate( throws HopException { Map patternCache = cache.computeIfAbsent(patternName, k -> new ConcurrentHashMap<>()); + Set patternLegacyDone = + legacyDone.computeIfAbsent(patternName, k -> ConcurrentHashMap.newKeySet()); + String oldKey = legacyKey == null ? null : legacyKey.get(); + if (oldKey != null && (oldKey.equals(sourceKey) || patternLegacyDone.contains(oldKey))) { + oldKey = null; + } String cached = patternCache.get(sourceKey); - if (cached != null) { + if (cached != null && oldKey == null) { return cached; } ensureOpen(); for (int attempt = 1; ; attempt++) { try { - String masked = lookup(patternName, sourceKey); - String oldKey = masked == null && legacyKey != null ? legacyKey.get() : null; - if (oldKey != null && !oldKey.equals(sourceKey)) { - masked = lookup(patternName, oldKey); - if (masked != null) { - rekey(patternName, oldKey, sourceKey); + String masked = cached != null ? cached : lookup(patternName, sourceKey); + if (oldKey != null) { + String oldMasked = lookup(patternName, oldKey); + if (oldMasked != null) { + if (masked == null) { + rekey(patternName, oldKey, sourceKey); + masked = oldMasked; + } else { + delete(patternName, oldKey); + } } } if (masked == null) { @@ -135,6 +153,9 @@ public synchronized String findOrCreate( } database.commit(); patternCache.put(sourceKey, masked); + if (oldKey != null) { + patternLegacyDone.add(oldKey); + } return masked; } catch (HopException e) { rollbackQuietly(); @@ -210,6 +231,7 @@ public synchronized int count(String patternName) throws HopException { @Override public synchronized void close() { cache.clear(); + legacyDone.clear(); if (database != null) { database.disconnect(); database = null; @@ -316,6 +338,13 @@ private void rekey(String patternName, String oldKey, String newKey) throws HopE new Object[] {newKey, patternName, oldKey}); } + private void delete(String patternName, String sourceKey) throws HopException { + exec( + "DELETE FROM " + mapTable + " WHERE " + quotedPattern + " = ? AND " + quotedSource + " = ?", + twoStrings(), + new Object[] {patternName, sourceKey}); + } + private String selectNextSql() { return "SELECT " + quotedNext + " FROM " + sequenceTable + " WHERE " + quotedPattern + " = ?"; } diff --git a/plugins/transforms/maskfields/src/main/java/org/apache/hop/pipeline/transforms/maskfields/store/IMaskingStore.java b/plugins/transforms/maskfields/src/main/java/org/apache/hop/pipeline/transforms/maskfields/store/IMaskingStore.java index 78685d56e9f..c2b90d44241 100644 --- a/plugins/transforms/maskfields/src/main/java/org/apache/hop/pipeline/transforms/maskfields/store/IMaskingStore.java +++ b/plugins/transforms/maskfields/src/main/java/org/apache/hop/pipeline/transforms/maskfields/store/IMaskingStore.java @@ -33,9 +33,9 @@ String findOrCreate(String patternName, String sourceKey, MaskAllocator allocato /** * Like {@link #findOrCreate(String, String, MaskAllocator)}, but also finds a mapping stored - * under the key an earlier version wrote, and moves it to {@code sourceKey}. The store only asks - * for that key when {@code sourceKey} is not found. A store that never kept mappings across - * versions ignores it. + * under the key an earlier version wrote, and moves it to {@code sourceKey}, or removes it when + * {@code sourceKey} already has a mapping. A store that never kept mappings across versions + * ignores it. */ default String findOrCreate( String patternName, String sourceKey, KeySupplier legacyKey, MaskAllocator allocator) diff --git a/plugins/transforms/maskfields/src/main/resources/org/apache/hop/pipeline/transforms/maskfields/messages/messages_en_US.properties b/plugins/transforms/maskfields/src/main/resources/org/apache/hop/pipeline/transforms/maskfields/messages/messages_en_US.properties index 7291516c693..98728217008 100644 --- a/plugins/transforms/maskfields/src/main/resources/org/apache/hop/pipeline/transforms/maskfields/messages/messages_en_US.properties +++ b/plugins/transforms/maskfields/src/main/resources/org/apache/hop/pipeline/transforms/maskfields/messages/messages_en_US.properties @@ -52,7 +52,7 @@ MaskFields.Check.PlainTextKeys=Masking pattern ''{0}'' has no hash secret, so it MaskFields.Error.FieldMissing=Field ''{0}'' is no longer in the input row. MaskFields.Error.Convert=Unable to write the masked value into field ''{0}'' with pattern ''{1}''. MaskFields.Error.BadSequence=Sequence start ''{0}'' on masking pattern ''{1}'' is not an integer. -MaskFields.Error.HashSecretUnresolved=The hash secret of masking pattern ''{0}'' is empty or holds a variable that is not set. +MaskFields.Error.HashSecretUnresolved=The hash secret of masking pattern ''{0}'' is empty, holds a variable that is not set, or does not decrypt. MaskFields.Error.OpenStore=Unable to open the mapping table for masking pattern ''{0}''. MaskingRules.SetEmpty=Set to blank only applies to a string field. diff --git a/plugins/transforms/maskfields/src/test/java/org/apache/hop/pipeline/transforms/maskfields/MaskingKeyTest.java b/plugins/transforms/maskfields/src/test/java/org/apache/hop/pipeline/transforms/maskfields/MaskingKeyTest.java index 10748449361..d57ddea2296 100644 --- a/plugins/transforms/maskfields/src/test/java/org/apache/hop/pipeline/transforms/maskfields/MaskingKeyTest.java +++ b/plugins/transforms/maskfields/src/test/java/org/apache/hop/pipeline/transforms/maskfields/MaskingKeyTest.java @@ -62,7 +62,11 @@ void trimAndIgnoreCaseAreOptional() throws Exception { void aHashSecretThatDoesNotResolveIsAnError() { Variables variables = new Variables(); variables.setVariable("EMPTY_SECRET", ""); - for (String secret : new String[] {"${NOT_SET}", "${EMPTY_SECRET}", "%%NOT_SET%%"}) { + variables.setVariable("BROKEN_SECRET", "Encrypted not-a-ciphertext"); + for (String secret : + new String[] { + "${NOT_SET}", "${EMPTY_SECRET}", "%%NOT_SET%%", "${BROKEN_SECRET}", "Encrypted " + }) { MaskingPattern pattern = databasePattern(secret); HopException e = assertThrows(HopException.class, () -> MaskingKey.forPattern(pattern, variables)); diff --git a/plugins/transforms/maskfields/src/test/java/org/apache/hop/pipeline/transforms/maskfields/MaskingRuntimeTest.java b/plugins/transforms/maskfields/src/test/java/org/apache/hop/pipeline/transforms/maskfields/MaskingRuntimeTest.java index e5627d77849..6f8994ab6fe 100644 --- a/plugins/transforms/maskfields/src/test/java/org/apache/hop/pipeline/transforms/maskfields/MaskingRuntimeTest.java +++ b/plugins/transforms/maskfields/src/test/java/org/apache/hop/pipeline/transforms/maskfields/MaskingRuntimeTest.java @@ -18,11 +18,15 @@ package org.apache.hop.pipeline.transforms.maskfields; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNotEquals; +import static org.junit.jupiter.api.Assertions.assertNull; import java.util.List; import java.util.UUID; +import org.apache.hop.core.database.DatabaseMeta; import org.apache.hop.core.row.RowMeta; import org.apache.hop.core.row.value.ValueMetaString; +import org.apache.hop.core.variables.Variables; import org.apache.hop.pipeline.transforms.maskfields.MaskingEngine.Binding; import org.junit.jupiter.api.Test; @@ -125,6 +129,23 @@ private static RowMeta stringRow(String name) { return rowMeta; } + @Test + void aManualUrlIsKeyedOnTheUrlAConnectionOpens() throws Exception { + DatabaseMeta databaseMeta = + new DatabaseMeta("mask", "H2", "Native", "", "unused", "", "sa", ""); + databaseMeta.setManualUrl("jdbc:h2:mem:${MASK_DB}"); + Variables one = new Variables(); + one.setVariable("MASK_DB", "one"); + Variables two = new Variables(); + two.setVariable("MASK_DB", "two"); + + String key = MaskingRuntime.databaseKey(one, databaseMeta, null, "mask_map"); + assertEquals("jdbc:h2:mem:one", key.substring(0, key.indexOf('\0'))); + assertNotEquals(key, MaskingRuntime.databaseKey(two, databaseMeta, null, "mask_map")); + // Unresolved, the URL does not say which database it opens. + assertNull(MaskingRuntime.databaseKey(new Variables(), databaseMeta, null, "mask_map")); + } + private static String uniqueId() { return "runtime-" + UUID.randomUUID(); } diff --git a/plugins/transforms/maskfields/src/test/java/org/apache/hop/pipeline/transforms/maskfields/store/DatabaseMaskingStoreTest.java b/plugins/transforms/maskfields/src/test/java/org/apache/hop/pipeline/transforms/maskfields/store/DatabaseMaskingStoreTest.java index e7850fb0adc..84ae5abcbed 100644 --- a/plugins/transforms/maskfields/src/test/java/org/apache/hop/pipeline/transforms/maskfields/store/DatabaseMaskingStoreTest.java +++ b/plugins/transforms/maskfields/src/test/java/org/apache/hop/pipeline/transforms/maskfields/store/DatabaseMaskingStoreTest.java @@ -216,6 +216,68 @@ void aPlainTextRowMovesToItsHashedKey() throws Exception { assertFalse(keys.contains("Matt")); } + @Test + void twoStoredSpellingsOfOneKeyLeaveNoPlainTextRow() throws Exception { + DatabaseMeta databaseMeta = h2(); + Variables variables = new Variables(); + DatabaseMaskingStore store = + new DatabaseMaskingStore((ILoggingObject) null, variables, databaseMeta, null, "mask_map"); + store.open(); + try { + // Rows written before the pattern ignored case. + assertEquals("first-name-1", allocate(store, "Matt")); + assertEquals("first-name-2", allocate(store, "MATT")); + } finally { + store.close(); + } + + DatabaseMaskingStore later = + new DatabaseMaskingStore((ILoggingObject) null, variables, databaseMeta, null, "mask_map"); + later.open(); + try { + assertEquals("first-name-1", allocate(later, "matt", "Matt")); + // The new key is cached now. The second spelling still has to leave the table. + assertEquals("first-name-1", allocate(later, "matt", "MATT")); + } finally { + later.close(); + } + assertEquals(List.of("matt"), sourceKeys(databaseMeta, variables)); + + DatabaseMaskingStore again = + new DatabaseMaskingStore((ILoggingObject) null, variables, databaseMeta, null, "mask_map"); + again.open(); + try { + assertEquals("first-name-1", allocate(again, "matt", "MATT")); + } finally { + again.close(); + } + assertEquals(List.of("matt"), sourceKeys(databaseMeta, variables)); + } + + @Test + void aManualUrlIsKeyedOnTheDatabaseItOpens() throws Exception { + String one = "mem:mask" + UUID.randomUUID().toString().replace("-", ""); + String two = "mem:mask" + UUID.randomUUID().toString().replace("-", ""); + DatabaseMeta databaseMeta = h2(); + databaseMeta.setManualUrl("jdbc:h2:${MASK_DB};DB_CLOSE_DELAY=-1"); + Variables first = new Variables(); + first.setVariable("MASK_DB", one); + Variables second = new Variables(); + second.setVariable("MASK_DB", two); + + MaskingRuntime.Lease lease = MaskingRuntime.getInstance().acquire("db-" + UUID.randomUUID()); + try { + DatabaseMaskingStore firstStore = lease.database(null, first, databaseMeta, null, "mask_map"); + DatabaseMaskingStore secondStore = + lease.database(null, second, databaseMeta, null, "mask_map"); + assertNotSame(firstStore, secondStore); + assertEquals("first-name-1", allocate(firstStore, "Matt")); + assertEquals("first-name-1", allocate(secondStore, "Ann")); + } finally { + lease.release(); + } + } + @Test void connectionsWithTheSameNameOnDifferentDatabasesGetTheirOwnStore() throws Exception { Variables variables = new Variables(); From a484438d9c5823a044c63cbe4011a2cb02fdad06 Mon Sep 17 00:00:00 2001 From: Bart Maertens Date: Fri, 9 Oct 2026 16:51:22 +0200 Subject: [PATCH 3/3] Initialize the Hop environment in MaskingRuntimeTest (#8791) --- .../pipeline/transforms/maskfields/MaskingRuntimeTest.java | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/plugins/transforms/maskfields/src/test/java/org/apache/hop/pipeline/transforms/maskfields/MaskingRuntimeTest.java b/plugins/transforms/maskfields/src/test/java/org/apache/hop/pipeline/transforms/maskfields/MaskingRuntimeTest.java index 6f8994ab6fe..deb5a1fe363 100644 --- a/plugins/transforms/maskfields/src/test/java/org/apache/hop/pipeline/transforms/maskfields/MaskingRuntimeTest.java +++ b/plugins/transforms/maskfields/src/test/java/org/apache/hop/pipeline/transforms/maskfields/MaskingRuntimeTest.java @@ -23,15 +23,22 @@ import java.util.List; import java.util.UUID; +import org.apache.hop.core.HopEnvironment; import org.apache.hop.core.database.DatabaseMeta; import org.apache.hop.core.row.RowMeta; import org.apache.hop.core.row.value.ValueMetaString; import org.apache.hop.core.variables.Variables; import org.apache.hop.pipeline.transforms.maskfields.MaskingEngine.Binding; +import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.Test; class MaskingRuntimeTest { + @BeforeAll + static void initHop() throws Exception { + HopEnvironment.init(); + } + @Test void memoryIsSharedInsideOneExecutionAndResetForTheNext() throws Exception { String executionId = uniqueId();