You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@hop.apache.org by mc...@apache.org on 2026/10/11 21:36:07 UTC
(hop) branch main updated: Fixes #8591 : Lint: correct the TRANS-002 description, check nested s… (#8852)
This is an automated email from the ASF dual-hosted git repository.
mattcasters pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/hop.git
The following commit(s) were added to refs/heads/main by this push:
new b0576f4bdc Fixes #8591 : Lint: correct the TRANS-002 description, check nested s… (#8852)
b0576f4bdc is described below
commit b0576f4bdcd50e94dd1312028518820325ac2d5c
Author: Bart Maertens <ba...@know.bi>
AuthorDate: Sun Oct 11 23:36:01 2026 +0200
Fixes #8591 : Lint: correct the TRANS-002 description, check nested s… (#8852)
* Fixes #8591 : Lint: correct the TRANS-002 description, check nested secrets, recognise every variable syntax, warn about unknown rule parameters
* Issue #8591 : Lint: add an integration test for nested secrets, the TRANS-002 wording and unknown rule parameters
---
.../modules/ROOT/pages/linting/lint-rules.adoc | 38 ++-
integration-tests/lint/core-rules/hop-lint.yml | 23 ++
.../lint/core-rules/nested-secrets.hpl | 288 +++++++++++++++++++++
integration-tests/lint/core-rules/orphan.hpl | 101 ++++++++
.../lint/main-0005-core-rule-pack.hwf | 200 ++++++++++++++
.../java/org/apache/hop/lint/CustomLintRule.java | 11 +
.../org/apache/hop/lint/CustomRuleExecutor.java | 154 ++++++++---
.../hop/lint/registry/YamlRulePackParser.java | 30 ++-
.../misc/lint/src/main/resources/hop-lint-core.yml | 12 +-
.../lint/src/main/resources/hop-lint.yml.example | 8 +-
.../apache/hop/lint/CoreRulePackReviewTest.java | 181 +++++++++++++
.../lint/registry/YamlRulePackParserTagsTest.java | 39 +++
12 files changed, 1031 insertions(+), 54 deletions(-)
diff --git a/docs/hop-user-manual/modules/ROOT/pages/linting/lint-rules.adoc b/docs/hop-user-manual/modules/ROOT/pages/linting/lint-rules.adoc
index 946cd07826..0b3eb71a04 100644
--- a/docs/hop-user-manual/modules/ROOT/pages/linting/lint-rules.adoc
+++ b/docs/hop-user-manual/modules/ROOT/pages/linting/lint-rules.adoc
@@ -101,7 +101,7 @@ Scope such a rule with `appliesTo`, naming the metadata key — the folder name
|is a collection with at least one entry
|`NO_HARDCODED`
-|is empty, or is a variable reference rather than a literal value
+|is empty, or takes its value from a variable (`${NAME}`, `%%NAME%%`) or a variable resolver (`#{vault:path:key}`) rather than holding a literal value. An `Encrypted` value is a literal value: Hop obfuscates it, and anyone with Hop and the same encoder key can read it back
|`MAX_VALUE`, `MIN_VALUE`, `EXACT_VALUE`
|compares as required against `conditionValue`
@@ -245,6 +245,32 @@ A rule aimed at `TRANSFORM` runs against every transform in the pipeline.
A scoped rule naming a field the transform does not have is reported as a configuration error rather than passing quietly, so that a typo in a field name is visible instead of looking like a clean result.
+== Parameters
+
+A few rules read extra settings from a `parameters:` block:
+
+[cols="1,3", options="header"]
+|===
+|Parameter |Read by
+
+|`fieldPatterns`
+|A `NO_HARDCODED` rule on `TRANSFORM` or `ACTION` whose `targetField` is `password`, `secret` or `credential`. The list of field name endings that hold a secret, replacing the default list
+
+|`blockingTransforms`
+|`isBlockingTransform`. The transform plugin ids that count as blocking, replacing the default list
+|===
+
+[source,yaml]
+----
+ SEC-002:
+ parameters:
+ fieldPatterns:
+ - "password"
+ - "apiKey"
+----
+
+Any other parameter is ignored, and a warning in the log names it, in a rule pack and in `hop-lint.yml` alike.
+
== Hop's own verify remarks
Every transform and action has a `check()` method of its own, the one behind the *Verify* button, and the linter reports what those find alongside its own rules.
@@ -378,6 +404,15 @@ Hop's core rule pack is deliberately small. A rule is enabled by default only wh
`TRANS-002` and `WORKFLOW-002` report on the transform or action itself, so the finding names it and the canvas can point at it.
The only element in a file is not an orphan — there is nothing for it to be disconnected from — and neither is one whose hops are all disabled: it has hops, and whether a disabled hop is a problem is what `STRUCT-003` asks.
+An orphaned action never runs, because nothing leads to it from Start.
+An orphaned transform does run: Hop starts every transform in a pipeline, connected or not.
+`TRANS-002` warns that it runs as a stream of its own, which is rarely what was meant.
+A pipeline that deliberately runs two independent streams can accept the finding on that transform, see <<accept-on-transform>>.
+
+`SEC-002` and `SEC-003` look at every text field of the transform or action whose name ends in one of the rule's `fieldPatterns`, such as `password`, `token` or `passphrase`.
+They also look inside the settings the transform or action stores in the file, such as the list of files PGP Decrypt Files decrypts, each with its own passphrase.
+`DB-001`, `SEC-002` and `SEC-003` count an `Encrypted` value as hardcoded.
+
Hop ships no narrowed `native` rule of its own.
A check that is simply wrong is fixed in the transform rather than silenced from a rule pack, which would leave it firing for everyone who presses *Verify*.
`appliesTo` and `messageKey` are there for a project that disagrees with a check the platform is right to ship.
@@ -510,6 +545,7 @@ Both are in *Tools -> Lint* as well.
A file covered by a broader pattern, `templates/**` rather than its own name, is left alone: which entry to change is a decision for you rather than a guess by the menu.
+[[accept-on-transform]]
== Accepting findings on a single transform or action
An exclusion covers a whole file, which is right for a template that is dynamic from end to end and too much for one that is only partly dynamic.
diff --git a/integration-tests/lint/core-rules/hop-lint.yml b/integration-tests/lint/core-rules/hop-lint.yml
new file mode 100644
index 0000000000..43bfc5a388
--- /dev/null
+++ b/integration-tests/lint/core-rules/hop-lint.yml
@@ -0,0 +1,23 @@
+# 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 #8591: no rule reads checkPasswords, so loading this file logs a warning naming it.
+# The override changes nothing else: SEC-002 keeps the pack's fieldPatterns.
+rules:
+ SEC-002:
+ parameters:
+ checkPasswords: false
diff --git a/integration-tests/lint/core-rules/nested-secrets.hpl b/integration-tests/lint/core-rules/nested-secrets.hpl
new file mode 100644
index 0000000000..9229e1b229
--- /dev/null
+++ b/integration-tests/lint/core-rules/nested-secrets.hpl
@@ -0,0 +1,288 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<!--
+
+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.
+
+-->
+<pipeline>
+ <info>
+ <name>nested-secrets</name>
+ <name_sync_with_filename>Y</name_sync_with_filename>
+ <description>Issue #8591: the Excel writer keeps its sheet password at file.password, one level down</description>
+ <extended_description/>
+ <pipeline_version/>
+ <pipeline_type>Normal</pipeline_type>
+ <parameters>
+ </parameters>
+ <capture_transform_performance>N</capture_transform_performance>
+ <transform_performance_capturing_delay>1000</transform_performance_capturing_delay>
+ <transform_performance_capturing_size_limit>100</transform_performance_capturing_size_limit>
+ <created_user>-</created_user>
+ <created_date>2026/10/10 12:00:00.000</created_date>
+ <modified_user>-</modified_user>
+ <modified_date>2026/10/10 12:00:00.000</modified_date>
+ </info>
+ <notepads>
+ </notepads>
+ <order>
+ <hop>
+ <from>Literal password</from>
+ <to>Encrypted password</to>
+ <enabled>Y</enabled>
+ </hop>
+ <hop>
+ <from>Encrypted password</from>
+ <to>Password from a Windows-style variable</to>
+ <enabled>Y</enabled>
+ </hop>
+ <hop>
+ <from>Password from a Windows-style variable</from>
+ <to>Password from a variable resolver</to>
+ <enabled>Y</enabled>
+ </hop>
+ </order>
+ <transform>
+ <name>Literal password</name>
+ <type>TypeExitExcelWriterTransform</type>
+ <description/>
+ <distribute>Y</distribute>
+ <custom_distribution/>
+ <copies>1</copies>
+ <partitioning>
+ <method>none</method>
+ <schema_name/>
+ </partitioning>
+ <add_to_result_filenames>Y</add_to_result_filenames>
+ <appendEmpty>0</appendEmpty>
+ <appendLines>N</appendLines>
+ <appendOffset>0</appendOffset>
+ <appendOmitHeader>N</appendOmitHeader>
+ <fields>
+ </fields>
+ <file>
+ <SpecifyFormat>N</SpecifyFormat>
+ <add_date>N</add_date>
+ <add_time>N</add_time>
+ <autosizecolums>N</autosizecolums>
+ <createParentFolder>Y</createParentFolder>
+ <do_not_open_newfile_init>Y</do_not_open_newfile_init>
+ <extension>xlsx</extension>
+ <filename_in_field>N</filename_in_field>
+ <if_file_exists>new</if_file_exists>
+ <if_sheet_exists>new</if_sheet_exists>
+ <name>${java.io.tmpdir}/core-rules</name>
+ <password>letmein</password>
+ <protect_sheet>Y</protect_sheet>
+ <protected_by>lint</protected_by>
+ <sheetname>Sheet1</sheetname>
+ <split>N</split>
+ <splitevery>0</splitevery>
+ <stream_data>N</stream_data>
+ </file>
+ <footer>N</footer>
+ <forceFormulaRecalculation>N</forceFormulaRecalculation>
+ <header>Y</header>
+ <leaveExistingStylesUnchanged>N</leaveExistingStylesUnchanged>
+ <makeSheetActive>Y</makeSheetActive>
+ <rowWritingMethod>overwrite</rowWritingMethod>
+ <startingCell>A1</startingCell>
+ <template>
+ <enabled>N</enabled>
+ <filename>template.xls</filename>
+ <hidden>N</hidden>
+ <sheet_enabled>N</sheet_enabled>
+ </template>
+ <attributes/>
+ <GUI>
+ <xloc>64</xloc>
+ <yloc>64</yloc>
+ </GUI>
+ </transform>
+ <transform>
+ <name>Encrypted password</name>
+ <type>TypeExitExcelWriterTransform</type>
+ <description/>
+ <distribute>Y</distribute>
+ <custom_distribution/>
+ <copies>1</copies>
+ <partitioning>
+ <method>none</method>
+ <schema_name/>
+ </partitioning>
+ <add_to_result_filenames>Y</add_to_result_filenames>
+ <appendEmpty>0</appendEmpty>
+ <appendLines>N</appendLines>
+ <appendOffset>0</appendOffset>
+ <appendOmitHeader>N</appendOmitHeader>
+ <fields>
+ </fields>
+ <file>
+ <SpecifyFormat>N</SpecifyFormat>
+ <add_date>N</add_date>
+ <add_time>N</add_time>
+ <autosizecolums>N</autosizecolums>
+ <createParentFolder>Y</createParentFolder>
+ <do_not_open_newfile_init>Y</do_not_open_newfile_init>
+ <extension>xlsx</extension>
+ <filename_in_field>N</filename_in_field>
+ <if_file_exists>new</if_file_exists>
+ <if_sheet_exists>new</if_sheet_exists>
+ <name>${java.io.tmpdir}/core-rules</name>
+ <password>Encrypted 2be98afc86aa7f2e4cb79ce10df90acde</password>
+ <protect_sheet>Y</protect_sheet>
+ <protected_by>lint</protected_by>
+ <sheetname>Sheet1</sheetname>
+ <split>N</split>
+ <splitevery>0</splitevery>
+ <stream_data>N</stream_data>
+ </file>
+ <footer>N</footer>
+ <forceFormulaRecalculation>N</forceFormulaRecalculation>
+ <header>Y</header>
+ <leaveExistingStylesUnchanged>N</leaveExistingStylesUnchanged>
+ <makeSheetActive>Y</makeSheetActive>
+ <rowWritingMethod>overwrite</rowWritingMethod>
+ <startingCell>A1</startingCell>
+ <template>
+ <enabled>N</enabled>
+ <filename>template.xls</filename>
+ <hidden>N</hidden>
+ <sheet_enabled>N</sheet_enabled>
+ </template>
+ <attributes/>
+ <GUI>
+ <xloc>256</xloc>
+ <yloc>64</yloc>
+ </GUI>
+ </transform>
+ <transform>
+ <name>Password from a Windows-style variable</name>
+ <type>TypeExitExcelWriterTransform</type>
+ <description/>
+ <distribute>Y</distribute>
+ <custom_distribution/>
+ <copies>1</copies>
+ <partitioning>
+ <method>none</method>
+ <schema_name/>
+ </partitioning>
+ <add_to_result_filenames>Y</add_to_result_filenames>
+ <appendEmpty>0</appendEmpty>
+ <appendLines>N</appendLines>
+ <appendOffset>0</appendOffset>
+ <appendOmitHeader>N</appendOmitHeader>
+ <fields>
+ </fields>
+ <file>
+ <SpecifyFormat>N</SpecifyFormat>
+ <add_date>N</add_date>
+ <add_time>N</add_time>
+ <autosizecolums>N</autosizecolums>
+ <createParentFolder>Y</createParentFolder>
+ <do_not_open_newfile_init>Y</do_not_open_newfile_init>
+ <extension>xlsx</extension>
+ <filename_in_field>N</filename_in_field>
+ <if_file_exists>new</if_file_exists>
+ <if_sheet_exists>new</if_sheet_exists>
+ <name>${java.io.tmpdir}/core-rules</name>
+ <password>%%XLS_PASSWORD%%</password>
+ <protect_sheet>Y</protect_sheet>
+ <protected_by>lint</protected_by>
+ <sheetname>Sheet1</sheetname>
+ <split>N</split>
+ <splitevery>0</splitevery>
+ <stream_data>N</stream_data>
+ </file>
+ <footer>N</footer>
+ <forceFormulaRecalculation>N</forceFormulaRecalculation>
+ <header>Y</header>
+ <leaveExistingStylesUnchanged>N</leaveExistingStylesUnchanged>
+ <makeSheetActive>Y</makeSheetActive>
+ <rowWritingMethod>overwrite</rowWritingMethod>
+ <startingCell>A1</startingCell>
+ <template>
+ <enabled>N</enabled>
+ <filename>template.xls</filename>
+ <hidden>N</hidden>
+ <sheet_enabled>N</sheet_enabled>
+ </template>
+ <attributes/>
+ <GUI>
+ <xloc>448</xloc>
+ <yloc>64</yloc>
+ </GUI>
+ </transform>
+ <transform>
+ <name>Password from a variable resolver</name>
+ <type>TypeExitExcelWriterTransform</type>
+ <description/>
+ <distribute>Y</distribute>
+ <custom_distribution/>
+ <copies>1</copies>
+ <partitioning>
+ <method>none</method>
+ <schema_name/>
+ </partitioning>
+ <add_to_result_filenames>Y</add_to_result_filenames>
+ <appendEmpty>0</appendEmpty>
+ <appendLines>N</appendLines>
+ <appendOffset>0</appendOffset>
+ <appendOmitHeader>N</appendOmitHeader>
+ <fields>
+ </fields>
+ <file>
+ <SpecifyFormat>N</SpecifyFormat>
+ <add_date>N</add_date>
+ <add_time>N</add_time>
+ <autosizecolums>N</autosizecolums>
+ <createParentFolder>Y</createParentFolder>
+ <do_not_open_newfile_init>Y</do_not_open_newfile_init>
+ <extension>xlsx</extension>
+ <filename_in_field>N</filename_in_field>
+ <if_file_exists>new</if_file_exists>
+ <if_sheet_exists>new</if_sheet_exists>
+ <name>${java.io.tmpdir}/core-rules</name>
+ <password>#{vault:secret/xls:password}</password>
+ <protect_sheet>Y</protect_sheet>
+ <protected_by>lint</protected_by>
+ <sheetname>Sheet1</sheetname>
+ <split>N</split>
+ <splitevery>0</splitevery>
+ <stream_data>N</stream_data>
+ </file>
+ <footer>N</footer>
+ <forceFormulaRecalculation>N</forceFormulaRecalculation>
+ <header>Y</header>
+ <leaveExistingStylesUnchanged>N</leaveExistingStylesUnchanged>
+ <makeSheetActive>Y</makeSheetActive>
+ <rowWritingMethod>overwrite</rowWritingMethod>
+ <startingCell>A1</startingCell>
+ <template>
+ <enabled>N</enabled>
+ <filename>template.xls</filename>
+ <hidden>N</hidden>
+ <sheet_enabled>N</sheet_enabled>
+ </template>
+ <attributes/>
+ <GUI>
+ <xloc>640</xloc>
+ <yloc>64</yloc>
+ </GUI>
+ </transform>
+ <transform_error_handling>
+ </transform_error_handling>
+ <attributes/>
+</pipeline>
diff --git a/integration-tests/lint/core-rules/orphan.hpl b/integration-tests/lint/core-rules/orphan.hpl
new file mode 100644
index 0000000000..b52811a2e9
--- /dev/null
+++ b/integration-tests/lint/core-rules/orphan.hpl
@@ -0,0 +1,101 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<!--
+
+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.
+
+-->
+<pipeline>
+ <info>
+ <name>orphan</name>
+ <name_sync_with_filename>Y</name_sync_with_filename>
+ <description>Issue #8591: one transform with no hops beside two connected ones</description>
+ <extended_description/>
+ <pipeline_version/>
+ <pipeline_type>Normal</pipeline_type>
+ <parameters>
+ </parameters>
+ <capture_transform_performance>N</capture_transform_performance>
+ <transform_performance_capturing_delay>1000</transform_performance_capturing_delay>
+ <transform_performance_capturing_size_limit>100</transform_performance_capturing_size_limit>
+ <created_user>-</created_user>
+ <created_date>2026/10/10 12:00:00.000</created_date>
+ <modified_user>-</modified_user>
+ <modified_date>2026/10/10 12:00:00.000</modified_date>
+ </info>
+ <notepads>
+ </notepads>
+ <order>
+ <hop>
+ <from>Connected source</from>
+ <to>Connected target</to>
+ <enabled>Y</enabled>
+ </hop>
+ </order>
+ <transform>
+ <name>Connected source</name>
+ <type>Dummy</type>
+ <description/>
+ <distribute>Y</distribute>
+ <custom_distribution/>
+ <copies>1</copies>
+ <partitioning>
+ <method>none</method>
+ <schema_name/>
+ </partitioning>
+ <attributes/>
+ <GUI>
+ <xloc>64</xloc>
+ <yloc>64</yloc>
+ </GUI>
+ </transform>
+ <transform>
+ <name>Connected target</name>
+ <type>Dummy</type>
+ <description/>
+ <distribute>Y</distribute>
+ <custom_distribution/>
+ <copies>1</copies>
+ <partitioning>
+ <method>none</method>
+ <schema_name/>
+ </partitioning>
+ <attributes/>
+ <GUI>
+ <xloc>256</xloc>
+ <yloc>64</yloc>
+ </GUI>
+ </transform>
+ <transform>
+ <name>Left over</name>
+ <type>Dummy</type>
+ <description/>
+ <distribute>Y</distribute>
+ <custom_distribution/>
+ <copies>1</copies>
+ <partitioning>
+ <method>none</method>
+ <schema_name/>
+ </partitioning>
+ <attributes/>
+ <GUI>
+ <xloc>64</xloc>
+ <yloc>192</yloc>
+ </GUI>
+ </transform>
+ <transform_error_handling>
+ </transform_error_handling>
+ <attributes/>
+</pipeline>
diff --git a/integration-tests/lint/main-0005-core-rule-pack.hwf b/integration-tests/lint/main-0005-core-rule-pack.hwf
new file mode 100644
index 0000000000..3fde3eedd0
--- /dev/null
+++ b/integration-tests/lint/main-0005-core-rule-pack.hwf
@@ -0,0 +1,200 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<!--
+
+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.
+
+-->
+<workflow>
+ <name>main-0005-core-rule-pack</name>
+ <name_sync_with_filename>Y</name_sync_with_filename>
+ <description>Issue #8591: SEC-002 reads nested settings and accepts every variable syntax Hop resolves, TRANS-002 reports a transform with no hops and says it still runs, and a parameter no rule reads is logged.</description>
+ <extended_description/>
+ <workflow_version/>
+ <created_user>-</created_user>
+ <created_date>2026/10/10 12:00:00.000</created_date>
+ <modified_user>-</modified_user>
+ <modified_date>2026/10/10 12:00:00.000</modified_date>
+ <parameters>
+ </parameters>
+ <actions>
+ <action>
+ <name>Start</name>
+ <description/>
+ <type>SPECIAL</type>
+ <attributes/>
+ <DayOfMonth>1</DayOfMonth>
+ <hour>12</hour>
+ <intervalMinutes>60</intervalMinutes>
+ <intervalSeconds>0</intervalSeconds>
+ <minutes>0</minutes>
+ <repeat>N</repeat>
+ <schedulerType>0</schedulerType>
+ <weekDay>1</weekDay>
+ <parallel>N</parallel>
+ <xloc>64</xloc>
+ <yloc>64</yloc>
+ <attributes_hac/>
+ </action>
+ <action>
+ <name>Lint the core-rules folder</name>
+ <description/>
+ <type>RUN_LINTER</type>
+ <attributes/>
+ <target>${PROJECT_HOME}/core-rules</target>
+ <include_metadata>N</include_metadata>
+ <config_file/>
+ <minimum_severity>INFO</minimum_severity>
+ <fail_on>NONE</fail_on>
+ <max_warnings/>
+ <baseline_file/>
+ <report_file>${PROJECT_HOME}/output/core-rules-findings.json</report_file>
+ <report_format>JSON</report_format>
+ <parallel>N</parallel>
+ <xloc>240</xloc>
+ <yloc>64</yloc>
+ <attributes_hac/>
+ </action>
+ <action>
+ <name>Check the findings</name>
+ <description/>
+ <type>SHELL</type>
+ <attributes/>
+ <filename/>
+ <work_directory>${PROJECT_HOME}</work_directory>
+ <arg_from_previous>N</arg_from_previous>
+ <exec_per_row>N</exec_per_row>
+ <set_logfile>N</set_logfile>
+ <logfile/>
+ <set_append_logfile>N</set_append_logfile>
+ <logext/>
+ <add_date>N</add_date>
+ <add_time>N</add_time>
+ <insertScript>Y</insertScript>
+ <script>#!/bin/bash
+# Note on ${} below: Hop resolves its own variables in this script before running it, so only Hop
+# variables are written with braces. Shell variables are written without them. The Windows-style
+# variable and the resolver in nested-secrets.hpl are not spelled out here for the same reason.
+
+set -u
+
+PROJECT_DIR="${PROJECT_HOME}"
+REPORT="$PROJECT_DIR/output/core-rules-findings.json"
+
+failures=0
+
+# $1 label, $2 actual, $3 expected
+expect() {
+ if [ "$2" != "$3" ]; then
+ echo "FAIL: $1: expected '$3', got '$2'"
+ failures=$((failures + 1))
+ else
+ echo "OK: $1 = $2"
+ fi
+}
+
+if [ ! -s "$REPORT" ]; then
+ echo "FAIL: the action wrote no report at $REPORT"
+ exit 1
+fi
+
+# One finding per line, so a rule id and the transform it names can be matched together.
+FINDINGS=$(tr -d '\n' < "$REPORT" | sed 's/}, {/\n/g')
+
+# $1 and $2 are fixed strings that must both be on the finding's line
+count() {
+ printf '%s\n' "$FINDINGS" | grep -F -- "$1" | grep -cF -- "$2"
+}
+
+echo "=== SEC-002 reads the password the Excel writer keeps at file.password ==="
+expect "literal password" \
+ "$(count '"ruleId" : "SEC-002"' "Transform 'Literal password' has hardcoded value in field 'file.password'")" "1"
+expect "Encrypted password" \
+ "$(count '"ruleId" : "SEC-002"' "Transform 'Encrypted password' has hardcoded value in field 'file.password'")" "1"
+expect "Windows-style variable" \
+ "$(count '"ruleId" : "SEC-002"' "Transform 'Password from a Windows-style variable'")" "0"
+expect "variable resolver" \
+ "$(count '"ruleId" : "SEC-002"' "Transform 'Password from a variable resolver'")" "0"
+
+echo "=== TRANS-002 reports only the transform with no hops, and says it still runs ==="
+expect "TRANS-002 findings" "$(count '"ruleId" : "TRANS-002"' '"ruleId"')" "1"
+expect "TRANS-002 on Left over" "$(count '"ruleId" : "TRANS-002"' '"name" : "Left over"')" "1"
+expect "TRANS-002 wording" "$(count '"ruleId" : "TRANS-002"' 'It still runs')" "1"
+
+echo "=== A parameter no rule reads is logged when hop-lint.yml is loaded ==="
+HOP_CLI="${user.dir}/hop"
+if [ ! -x "$HOP_CLI" ]; then
+ echo "FAIL: no hop CLI at $HOP_CLI"
+ exit 1
+fi
+export HOP_CONFIG_FOLDER="$PROJECT_DIR"
+CLI_LOG="$PROJECT_DIR/output/core-rules-cli.log"
+"$HOP_CLI" lint "$PROJECT_DIR/core-rules" --fail-on NONE --format json \
+ --output "$PROJECT_DIR/output/core-rules-findings-cli.json" > "$CLI_LOG" 2>&1
+expect "unknown parameter warning" \
+ "$(grep -cF "has an unknown parameter 'checkPasswords', which is ignored." "$CLI_LOG")" "1"
+
+if [ $failures -gt 0 ]; then
+ echo "$failures check(s) failed."
+ exit 1
+fi
+echo "All checks passed."
+exit 0
+</script>
+ <loglevel>Basic</loglevel>
+ <nr>0</nr>
+ <parallel>N</parallel>
+ <xloc>448</xloc>
+ <yloc>64</yloc>
+ <attributes_hac/>
+ </action>
+ <action>
+ <name>Success</name>
+ <description/>
+ <type>SUCCESS</type>
+ <attributes/>
+ <parallel>N</parallel>
+ <xloc>640</xloc>
+ <yloc>64</yloc>
+ <attributes_hac/>
+ </action>
+ </actions>
+ <hops>
+ <hop>
+ <from>Start</from>
+ <to>Lint the core-rules folder</to>
+ <enabled>Y</enabled>
+ <evaluation>Y</evaluation>
+ <unconditional>Y</unconditional>
+ </hop>
+ <hop>
+ <from>Lint the core-rules folder</from>
+ <to>Check the findings</to>
+ <enabled>Y</enabled>
+ <evaluation>Y</evaluation>
+ <unconditional>N</unconditional>
+ </hop>
+ <hop>
+ <from>Check the findings</from>
+ <to>Success</to>
+ <enabled>Y</enabled>
+ <evaluation>Y</evaluation>
+ <unconditional>N</unconditional>
+ </hop>
+ </hops>
+ <notepads>
+ </notepads>
+ <attributes/>
+</workflow>
diff --git a/plugins/misc/lint/src/main/java/org/apache/hop/lint/CustomLintRule.java b/plugins/misc/lint/src/main/java/org/apache/hop/lint/CustomLintRule.java
index 69b337de63..3c367ac14b 100644
--- a/plugins/misc/lint/src/main/java/org/apache/hop/lint/CustomLintRule.java
+++ b/plugins/misc/lint/src/main/java/org/apache/hop/lint/CustomLintRule.java
@@ -21,6 +21,7 @@ import java.util.HashMap;
import java.util.LinkedHashMap;
import java.util.List;
import java.util.Map;
+import java.util.Set;
import java.util.UUID;
import org.apache.hop.lint.registry.RulePackIds;
import org.apache.hop.lint.registry.RulePackOwner;
@@ -28,6 +29,16 @@ import org.apache.hop.lint.registry.RulePackOwner;
/** Represents a user-defined linting rule */
public class CustomLintRule {
+ /** The field name patterns a hardcoded-secret rule looks for, replacing the default list. */
+ public static final String PARAMETER_FIELD_PATTERNS = "fieldPatterns";
+
+ /** The transform plugin ids {@code isBlockingTransform} counts, replacing the default list. */
+ public static final String PARAMETER_BLOCKING_TRANSFORMS = "blockingTransforms";
+
+ /** Every parameter the linter reads. A rule pack or hop-lint.yml naming another is warned. */
+ public static final Set<String> PARAMETERS =
+ Set.of(PARAMETER_FIELD_PATTERNS, PARAMETER_BLOCKING_TRANSFORMS);
+
private String id;
private String name;
private String description;
diff --git a/plugins/misc/lint/src/main/java/org/apache/hop/lint/CustomRuleExecutor.java b/plugins/misc/lint/src/main/java/org/apache/hop/lint/CustomRuleExecutor.java
index 6bef6b582c..06c1170eed 100644
--- a/plugins/misc/lint/src/main/java/org/apache/hop/lint/CustomRuleExecutor.java
+++ b/plugins/misc/lint/src/main/java/org/apache/hop/lint/CustomRuleExecutor.java
@@ -22,9 +22,11 @@ import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collection;
import java.util.Collections;
+import java.util.IdentityHashMap;
import java.util.LinkedHashMap;
import java.util.List;
import java.util.Map;
+import java.util.Set;
import java.util.TreeSet;
import java.util.regex.Pattern;
import org.apache.hop.core.database.DatabaseMeta;
@@ -765,7 +767,8 @@ public class CustomRuleExecutor {
*/
private static List<String> getBlockingTransformPlugins(CustomLintRule rule) {
if (rule != null && rule.getAdditionalParameters() != null) {
- Object configured = rule.getAdditionalParameters().get("blockingTransforms");
+ Object configured =
+ rule.getAdditionalParameters().get(CustomLintRule.PARAMETER_BLOCKING_TRANSFORMS);
if (configured instanceof List) {
@SuppressWarnings("unchecked")
List<String> ids = (List<String>) configured;
@@ -1072,36 +1075,26 @@ public class CustomRuleExecutor {
return results;
}
- // Get field patterns from rule parameters or use defaults
List<String> fieldPatterns = getPasswordFieldPatterns(rule);
-
- // Check all password-related fields
- Class<?> clazz = transformOrAction.getClass();
- for (Field field : getAllFields(clazz)) {
- for (String pattern : fieldPatterns) {
- if (namesASecret(field, pattern)) {
- try {
- field.setAccessible(true);
- Object value = field.get(transformOrAction);
- if (value != null && !Utils.isEmpty(value.toString())) {
- String strValue = value.toString();
- // Check if it's hardcoded (not a variable)
- if (!isVariable(strValue)) {
- String message =
- String.format(
- "%s '%s' has hardcoded value in field '%s'. Consider using a variable instead (e.g., ${%s})",
- rule.getTarget() == RuleTarget.TRANSFORM ? "Transform" : "Action",
- objectName,
- field.getName(),
- field.getName().toUpperCase().replaceAll("[^A-Z0-9]", "_"));
- results.add(createResult(rule, message, fileName, hopObject));
- }
- }
- } catch (Exception e) {
- log.logDetailed("Error checking field " + field.getName() + ": " + e.getMessage());
- }
- }
- }
+ Map<String, Field> hardcoded = new LinkedHashMap<>();
+ collectHardcodedSecrets(
+ transformOrAction,
+ "",
+ fieldPatterns,
+ Collections.newSetFromMap(new IdentityHashMap<>()),
+ 0,
+ hardcoded);
+
+ for (Map.Entry<String, Field> entry : hardcoded.entrySet()) {
+ String fieldName = entry.getValue().getName();
+ String message =
+ String.format(
+ "%s '%s' has hardcoded value in field '%s'. Consider using a variable instead (e.g., ${%s})",
+ rule.getTarget() == RuleTarget.TRANSFORM ? "Transform" : "Action",
+ objectName,
+ entry.getKey(),
+ fieldName.toUpperCase().replaceAll("[^A-Z0-9]", "_"));
+ results.add(createResult(rule, message, fileName, hopObject));
}
} catch (Exception e) {
@@ -1141,6 +1134,82 @@ public class CustomRuleExecutor {
return field.getName().toLowerCase().endsWith(pattern.trim().toLowerCase());
}
+ /** How deep {@link #collectHardcodedSecrets} follows settings nested in settings. */
+ private static final int MAX_SECRET_DEPTH = 4;
+
+ /**
+ * Collect the fields holding a hardcoded secret, keyed by their path from the transform or
+ * action.
+ *
+ * <p>A secret is not always a field of the transform or action itself. PGP Decrypt Files keeps a
+ * passphrase on every file it decrypts, at {@code filesToDecrypt[0].passphrase}. Only the
+ * properties Hop stores in the file ({@link HopMetadataProperty}) are followed, which keeps the
+ * walk out of references such as the parent transform and the pipeline behind it.
+ */
+ private static void collectHardcodedSecrets(
+ Object holder,
+ String path,
+ List<String> fieldPatterns,
+ Set<Object> visited,
+ int depth,
+ Map<String, Field> hardcoded) {
+ if (holder == null || !visited.add(holder)) {
+ return;
+ }
+ for (Field field : getAllFields(holder.getClass())) {
+ if (Modifier.isStatic(field.getModifiers())) {
+ continue;
+ }
+ try {
+ if (fieldPatterns.stream().anyMatch(pattern -> namesASecret(field, pattern))) {
+ field.setAccessible(true);
+ Object value = field.get(holder);
+ if (value != null && !Utils.isEmpty(value.toString()) && !isVariable(value.toString())) {
+ hardcoded.put(path + field.getName(), field);
+ }
+ } else if (depth < MAX_SECRET_DEPTH
+ && field.isAnnotationPresent(HopMetadataProperty.class)) {
+ field.setAccessible(true);
+ Object value = field.get(holder);
+ String nestedPath = path + field.getName();
+ if (value instanceof Collection<?> collection) {
+ int i = 0;
+ for (Object element : collection) {
+ walkInto(
+ element, nestedPath + "[" + i++ + "].", fieldPatterns, visited, depth, hardcoded);
+ }
+ } else if (value instanceof Object[] array) {
+ for (int i = 0; i < array.length; i++) {
+ walkInto(
+ array[i], nestedPath + "[" + i + "].", fieldPatterns, visited, depth, hardcoded);
+ }
+ } else {
+ walkInto(value, nestedPath + ".", fieldPatterns, visited, depth, hardcoded);
+ }
+ }
+ } catch (Exception e) {
+ log.logDetailed("Error checking field " + path + field.getName() + ": " + e.getMessage());
+ }
+ }
+ }
+
+ /** Follow a nested value when it is a settings object rather than a plain value. */
+ private static void walkInto(
+ Object value,
+ String path,
+ List<String> fieldPatterns,
+ Set<Object> visited,
+ int depth,
+ Map<String, Field> hardcoded) {
+ if (value == null
+ || value instanceof Enum<?>
+ || value.getClass().isArray()
+ || value.getClass().getName().startsWith("java.")) {
+ return;
+ }
+ collectHardcodedSecrets(value, path, fieldPatterns, visited, depth + 1, hardcoded);
+ }
+
/**
* Whether the value a clause reads is a secret, and so must stay out of the finding.
*
@@ -1227,7 +1296,8 @@ public class CustomRuleExecutor {
List<String> defaultPatterns = DEFAULT_SECRET_FIELD_PATTERNS;
if (rule.getAdditionalParameters() != null) {
- Object patternsObj = rule.getAdditionalParameters().get("fieldPatterns");
+ Object patternsObj =
+ rule.getAdditionalParameters().get(CustomLintRule.PARAMETER_FIELD_PATTERNS);
if (patternsObj instanceof List) {
@SuppressWarnings("unchecked")
List<String> patterns = (List<String>) patternsObj;
@@ -1240,19 +1310,17 @@ public class CustomRuleExecutor {
return defaultPatterns;
}
- /** Check if a string is a Hop variable (enclosed in ${...}) */
- private static boolean isVariable(String value) {
- if (Utils.isEmpty(value)) {
- return false;
- }
-
- // Check if it's a simple variable like ${VAR_NAME}
- if (value.startsWith("${") && value.endsWith("}")) {
- return true;
- }
+ /**
+ * A reference Hop resolves when it runs: a variable written as {@code ${NAME}} or {@code
+ * %%NAME%%}, or a variable resolver such as {@code #{vault:path:key}}. {@code $[..]} is not one:
+ * it spells literal characters in hex.
+ */
+ private static final Pattern VARIABLE_REFERENCE =
+ Pattern.compile("\\$\\{[^}]+}|%%[^%]+%%|#\\{[^}]+}");
- // Check if it contains variables (might be mixed with other text)
- return value.contains("${") && value.contains("}");
+ /** Whether the value takes something from a variable or resolver rather than the file. */
+ static boolean isVariable(String value) {
+ return !Utils.isEmpty(value) && VARIABLE_REFERENCE.matcher(value).find();
}
/** Evaluate a condition against a field value */
diff --git a/plugins/misc/lint/src/main/java/org/apache/hop/lint/registry/YamlRulePackParser.java b/plugins/misc/lint/src/main/java/org/apache/hop/lint/registry/YamlRulePackParser.java
index 979a60e5c7..d93d1bf1a9 100644
--- a/plugins/misc/lint/src/main/java/org/apache/hop/lint/registry/YamlRulePackParser.java
+++ b/plugins/misc/lint/src/main/java/org/apache/hop/lint/registry/YamlRulePackParser.java
@@ -309,6 +309,7 @@ public final class YamlRulePackParser {
projectRules.add(rule);
} else {
warnUnknownKeys(ruleId, location, ruleData, OVERRIDE_KEYS);
+ warnUnknownParameters(ruleId, location, ruleData.get("parameters"));
overlays.put(
ruleId, ProjectYamlOverlay.ProjectRuleOverlay.fromMap(ruleId, ruleData, location));
}
@@ -553,6 +554,7 @@ public final class YamlRulePackParser {
}
applyDocumentation(customRule, ruleData, location);
warnUnknownKeys(ruleId, location, ruleData, CUSTOM_RULE_KEYS);
+ warnUnknownParameters(ruleId, location, parameters);
return customRule;
}
@@ -636,6 +638,30 @@ public final class YamlRulePackParser {
*/
static List<String> unknownKeyWarnings(
String ruleId, String location, Map<String, Object> ruleData, Set<String> knownKeys) {
+ return unknownNameWarnings(ruleId, location, ruleData, knownKeys, "key");
+ }
+
+ /**
+ * One warning for each parameter on the rule that the linter does not read.
+ *
+ * <p>A parameter nothing reads looks like configuration and changes nothing. DB-001 used to carry
+ * {@code checkPasswords} and {@code checkUsernames}, which no code ever looked at.
+ *
+ * @param parameters the rule's {@code parameters:} block, or null
+ */
+ static List<String> unknownParameterWarnings(String ruleId, String location, Object parameters) {
+ if (!(parameters instanceof Map<?, ?> map)) {
+ return new ArrayList<>();
+ }
+ return unknownNameWarnings(ruleId, location, map, CustomLintRule.PARAMETERS, "parameter");
+ }
+
+ private static void warnUnknownParameters(String ruleId, String location, Object parameters) {
+ unknownParameterWarnings(ruleId, location, parameters).forEach(YamlRulePackParser::warn);
+ }
+
+ private static List<String> unknownNameWarnings(
+ String ruleId, String location, Map<?, ?> ruleData, Set<String> knownKeys, String what) {
List<String> warnings = new ArrayList<>();
if (ruleData == null) {
return warnings;
@@ -650,7 +676,9 @@ public final class YamlRulePackParser {
.append(ruleId)
.append("' ")
.append(location)
- .append(" has an unknown key '")
+ .append(" has an unknown ")
+ .append(what)
+ .append(" '")
.append(name)
.append("', which is ignored.");
String closest = RuleRegistry.closestId(name, knownKeys);
diff --git a/plugins/misc/lint/src/main/resources/hop-lint-core.yml b/plugins/misc/lint/src/main/resources/hop-lint-core.yml
index 629626f6e6..2b2b032b1f 100644
--- a/plugins/misc/lint/src/main/resources/hop-lint-core.yml
+++ b/plugins/misc/lint/src/main/resources/hop-lint-core.yml
@@ -53,7 +53,7 @@ rules:
targetField: password
condition: NO_HARDCODED
name: "Hardcoded Database Password"
- description: "Database connections must take the password from a variable rather than storing it in the project"
+ description: "Database connections must take the password from a variable rather than storing it in the project, encrypted or not"
SEC-002:
type: custom
@@ -63,7 +63,7 @@ rules:
targetField: password
condition: NO_HARDCODED
name: "Hardcoded Password or Secret in Transform"
- description: "Transforms must not store passwords, tokens or API keys in the project; use a variable instead"
+ description: "Transforms must not store passwords, tokens or API keys in the project, encrypted or not; use a variable instead"
parameters:
fieldPatterns:
- "password"
@@ -76,6 +76,7 @@ rules:
- "apiKey"
- "apikey"
- "secretAccessKey"
+ - "passphrase"
- "token"
- "accessToken"
- "authToken"
@@ -88,7 +89,7 @@ rules:
targetField: password
condition: NO_HARDCODED
name: "Hardcoded Password or Secret in Action"
- description: "Workflow actions must not store passwords, tokens or API keys in the project; use a variable instead"
+ description: "Workflow actions must not store passwords, tokens or API keys in the project, encrypted or not; use a variable instead"
parameters:
fieldPatterns:
- "password"
@@ -101,6 +102,7 @@ rules:
- "apiKey"
- "apikey"
- "secretAccessKey"
+ - "passphrase"
- "token"
- "accessToken"
- "authToken"
@@ -109,6 +111,8 @@ rules:
# A pipeline holding one transform and nothing else is not orphaned — there is nothing for it
# to be disconnected from — and neither is a transform whose hops are all disabled: it has
# hops, and whether a disabled hop is a problem is what STRUCT-003 is for.
+ # Such a transform still runs: Hop starts every transform in a pipeline, connected or not. The
+ # warning is that it runs as a stream of its own, which is rarely what was meant.
TRANS-002:
type: custom
enabled: true
@@ -117,7 +121,7 @@ rules:
targetField: isOrphaned
condition: MUST_BE_FALSE
name: "Orphaned Transform"
- description: "This transform has no incoming or outgoing hop, so it never executes"
+ description: "This transform has no incoming or outgoing hop. It still runs, as a separate stream next to the rest of the pipeline"
WORKFLOW-002:
type: custom
diff --git a/plugins/misc/lint/src/main/resources/hop-lint.yml.example b/plugins/misc/lint/src/main/resources/hop-lint.yml.example
index 834595f17f..7086231191 100644
--- a/plugins/misc/lint/src/main/resources/hop-lint.yml.example
+++ b/plugins/misc/lint/src/main/resources/hop-lint.yml.example
@@ -22,21 +22,19 @@
# Use this file for per-project overrides only.
rules:
- TRANS-001:
+ # Switch on a rule the core pack ships disabled, and set its ceiling
+ STRUCT-001:
enabled: true
severity: WARNING
- parameters:
- maxTransforms: 20
+ conditionValue: "20"
TRANS-002:
enabled: true
severity: WARNING
- parameters: {}
DB-001:
enabled: true
severity: ERROR
- parameters: {}
# Hop's own transform and action checks. A check that reports an error code
# is reported under that code, and answers to HOP-CHECK as well, so a rule or
diff --git a/plugins/misc/lint/src/test/java/org/apache/hop/lint/CoreRulePackReviewTest.java b/plugins/misc/lint/src/test/java/org/apache/hop/lint/CoreRulePackReviewTest.java
new file mode 100644
index 0000000000..7323b2b046
--- /dev/null
+++ b/plugins/misc/lint/src/test/java/org/apache/hop/lint/CoreRulePackReviewTest.java
@@ -0,0 +1,181 @@
+/*
+ * 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.lint;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+import java.util.ArrayList;
+import java.util.List;
+import org.apache.hop.core.HopEnvironment;
+import org.apache.hop.core.database.DatabaseMeta;
+import org.apache.hop.core.encryption.Encr;
+import org.apache.hop.lint.registry.RuleRegistry;
+import org.apache.hop.metadata.api.HopMetadataProperty;
+import org.apache.hop.pipeline.transform.BaseTransformMeta;
+import org.apache.hop.pipeline.transform.TransformMeta;
+import org.junit.jupiter.api.BeforeAll;
+import org.junit.jupiter.api.Test;
+
+/**
+ * The core rule pack against the code behind each rule.
+ *
+ * @see <a href="https://github.com/apache/hop/issues/8591">#8591</a>
+ */
+public class CoreRulePackReviewTest {
+
+ @BeforeAll
+ static void initHop() throws Exception {
+ HopEnvironment.init();
+ }
+
+ private static CustomLintRule coreRule(String id) {
+ CustomLintRule rule =
+ RuleRegistry.getInstance().resolve(null).getRules().stream()
+ .filter(r -> id.equals(r.generateRuleId()))
+ .findFirst()
+ .orElseThrow()
+ .copy();
+ rule.setEnabled(true);
+ return rule;
+ }
+
+ // ------------------------------------------------------------------ TRANS-002
+
+ /** Hop starts a transform that has no hops, so the rule must not say it never runs. */
+ @Test
+ public void trans002DoesNotClaimTheTransformNeverRuns() {
+ String description = coreRule("TRANS-002").getDescription();
+
+ assertFalse(description.contains("never executes"), description);
+ assertTrue(description.contains("still runs"), description);
+ }
+
+ // ------------------------------------------------------------------ SEC-002 nested settings
+
+ /** One file a transform reads, with a passphrase of its own, as PGP Decrypt Files has. */
+ public static class FileEntry {
+ @HopMetadataProperty private String fileName = "/data/in.pgp";
+ @HopMetadataProperty private String passphrase;
+
+ FileEntry(String passphrase) {
+ this.passphrase = passphrase;
+ }
+ }
+
+ /** Settings grouped in an object, as Text File Output's fileSettings are. */
+ public static class ConnectionSettings {
+ @HopMetadataProperty private String host = "example.org";
+ @HopMetadataProperty private String password = "letmein";
+ }
+
+ public static class NestedSecretsMeta extends BaseTransformMeta {
+ @HopMetadataProperty private ConnectionSettings settings = new ConnectionSettings();
+
+ @HopMetadataProperty
+ private List<FileEntry> files =
+ new ArrayList<>(List.of(new FileEntry("s3cr3t"), new FileEntry("${PGP_PASSPHRASE}")));
+
+ /** Not stored in the file, so not followed: a reference rather than a setting. */
+ private ConnectionSettings cached = new ConnectionSettings();
+ }
+
+ private static List<String> sec002Messages(BaseTransformMeta meta) {
+ TransformMeta transformMeta = new TransformMeta("Decrypt", "a transform", meta);
+ return CustomRuleExecutor.executeRule(coreRule("SEC-002"), transformMeta, "/tmp/p.hpl").stream()
+ .map(LintResult::getMessage)
+ .toList();
+ }
+
+ @Test
+ public void sec002LooksInsideTheSettingsATransformStores() {
+ List<String> messages = sec002Messages(new NestedSecretsMeta());
+
+ assertEquals(2, messages.size(), "unexpected findings: " + messages);
+ assertTrue(
+ messages.stream().anyMatch(m -> m.contains("field 'settings.password'")), "" + messages);
+ assertTrue(
+ messages.stream().anyMatch(m -> m.contains("field 'files[0].passphrase'")), "" + messages);
+ }
+
+ /** A field whose name ends in two patterns, such as accessToken, is one finding, not two. */
+ public static class AccessTokenMeta extends BaseTransformMeta {
+ private String accessToken = "abc";
+ }
+
+ @Test
+ public void aFieldMatchingTwoPatternsIsReportedOnce() {
+ assertEquals(1, sec002Messages(new AccessTokenMeta()).size());
+ }
+
+ // ------------------------------------------------------------------ variable syntaxes
+
+ @Test
+ public void everyVariableSyntaxHopResolvesCountsAsAVariable() {
+ assertTrue(CustomRuleExecutor.isVariable("${DB_PASSWORD}"));
+ assertTrue(CustomRuleExecutor.isVariable("%%DB_PASSWORD%%"));
+ assertTrue(CustomRuleExecutor.isVariable("#{vault:secret/data/db:password}"));
+ assertTrue(CustomRuleExecutor.isVariable("prefix-${SUFFIX}"));
+
+ assertFalse(CustomRuleExecutor.isVariable("letmein"));
+ assertFalse(CustomRuleExecutor.isVariable("$[6C,65,74]"), "hex spells a literal value");
+ assertFalse(CustomRuleExecutor.isVariable("100%"));
+ assertFalse(CustomRuleExecutor.isVariable("pa}ss${"));
+ }
+
+ public static class WindowsVariableMeta extends BaseTransformMeta {
+ private String password = "%%DB_PASSWORD%%";
+ private String apiKey = "#{vault:secret/data/api:key}";
+ }
+
+ @Test
+ public void sec002AcceptsWindowsStyleVariablesAndResolvers() {
+ assertEquals(List.of(), sec002Messages(new WindowsVariableMeta()));
+ }
+
+ // ------------------------------------------------------------------ Encrypted values
+
+ private static List<LintResult> db001(String password) {
+ DatabaseMeta databaseMeta = new DatabaseMeta();
+ databaseMeta.setName("CRM");
+ databaseMeta.setPassword(password);
+ return CustomRuleExecutor.executeRule(
+ coreRule("DB-001"), databaseMeta, "/tmp/metadata/rdbms/CRM.json");
+ }
+
+ /**
+ * Hop decrypts a password field when it loads the file, so the linter mostly sees the plain
+ * value. Where it does see an Encrypted one, that is reported as well, and the rule says so.
+ */
+ @Test
+ public void db001ReportsAnEncryptedPassword() {
+ String encrypted = Encr.encryptPasswordIfNotUsingVariables("secret123");
+ List<LintResult> results = db001(encrypted);
+
+ assertEquals(1, results.size());
+ String message = results.get(0).getMessage();
+ assertFalse(
+ message.contains(encrypted.substring(Encr.PASSWORD_ENCRYPTED_PREFIX.length())), message);
+ assertTrue(coreRule("DB-001").getDescription().contains("encrypted or not"));
+ }
+
+ @Test
+ public void db001AcceptsAWindowsStyleVariable() {
+ assertEquals(List.of(), db001("%%DB_PASSWORD%%"));
+ }
+}
diff --git a/plugins/misc/lint/src/test/java/org/apache/hop/lint/registry/YamlRulePackParserTagsTest.java b/plugins/misc/lint/src/test/java/org/apache/hop/lint/registry/YamlRulePackParserTagsTest.java
index 917c6322ac..89d03dcc8e 100644
--- a/plugins/misc/lint/src/test/java/org/apache/hop/lint/registry/YamlRulePackParserTagsTest.java
+++ b/plugins/misc/lint/src/test/java/org/apache/hop/lint/registry/YamlRulePackParserTagsTest.java
@@ -291,6 +291,41 @@ public class YamlRulePackParserTagsTest {
YamlRulePackParser.OVERRIDE_KEYS));
}
+ /**
+ * DB-001 carried checkPasswords and checkUsernames, which no code read, so they looked like
+ * settings and changed nothing.
+ *
+ * @see <a href="https://github.com/apache/hop/issues/8591">#8591</a>
+ */
+ @Test
+ public void anUnknownParameterIsReportedWithTheClosestKnownOne() {
+ Map<String, Object> parameters = new LinkedHashMap<>();
+ parameters.put("checkPasswords", true);
+ parameters.put("fieldPattern", List.of("password"));
+ parameters.put("blockingTransforms", List.of("SortRows"));
+
+ List<String> warnings =
+ YamlRulePackParser.unknownParameterWarnings(
+ "DB-001", YamlRulePackParser.inPack("acme"), parameters);
+
+ assertEquals(
+ List.of(
+ "Warning: rule 'DB-001' in pack 'acme' has an unknown parameter 'checkPasswords',"
+ + " which is ignored.",
+ "Warning: rule 'DB-001' in pack 'acme' has an unknown parameter 'fieldPattern',"
+ + " which is ignored. Did you mean fieldPatterns?"),
+ warnings);
+ }
+
+ @Test
+ public void noParametersProduceNoWarning() {
+ assertTrue(
+ YamlRulePackParser.unknownParameterWarnings("DB-001", "in pack 'acme'", null).isEmpty());
+ assertTrue(
+ YamlRulePackParser.unknownParameterWarnings("DB-001", "in pack 'acme'", Map.of())
+ .isEmpty());
+ }
+
/** Every key the core pack uses has to be a known key, or Hop warns about its own rules. */
@Test
public void theCorePackUsesOnlyKnownKeys() throws Exception {
@@ -310,6 +345,10 @@ public class YamlRulePackParserTagsTest {
List.of(),
YamlRulePackParser.unknownKeyWarnings(
entry.getKey(), YamlRulePackParser.inPack("hop-core"), ruleData, known));
+ assertEquals(
+ List.of(),
+ YamlRulePackParser.unknownParameterWarnings(
+ entry.getKey(), YamlRulePackParser.inPack("hop-core"), ruleData.get("parameters")));
}
}
}