You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@hop.apache.org by "bamaer (via GitHub)" <gi...@apache.org> on 2026/10/03 05:51:23 UTC
[PR] Fixes #8730 : Keep the values of password fields out of lint findings (hop)
bamaer opened a new pull request, #8737:
URL: https://github.com/apache/hop/pull/8737
**Please** add a meaningful description for your change here
------------------------
Thank you for your contribution! Follow this checklist to help us incorporate your contribution quickly and easily:
- [x] Run `mvn clean install apache-rat:check` to make sure basic checks pass. A more thorough check will be performed on your pull request automatically.
- [x] If you have a group of commits related to the same change, please squash your commits into one and force push your branch using `git rebase -i`.
- [x] Mention the appropriate issue in your description (for example: `addresses #123`), if applicable.
To make clear that you license your contribution under the [Apache License Version 2.0, January 2004](http://www.apache.org/licenses/LICENSE-2.0)
you have to acknowledge this by using the following check-box.
- [x] I hereby declare this contribution to be licensed under the [Apache License Version 2.0, January 2004](http://www.apache.org/licenses/LICENSE-2.0)
- [ ] In any other case, please file an [Apache Individual Contributor License Agreement](https://www.apache.org/licenses/icla.pdf).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@hop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fixes #8730 : Keep the values of password fields out of lint findings (hop)
Posted by "mattcasters (via GitHub)" <gi...@apache.org>.
mattcasters commented on code in PR #8737:
URL: https://github.com/apache/hop/pull/8737#discussion_r4172761012
##########
plugins/misc/lint/src/main/java/org/apache/hop/lint/CustomRuleExecutor.java:
##########
@@ -940,23 +952,54 @@ private static boolean namesASecret(Field field, String pattern) {
return field.getName().toLowerCase().endsWith(pattern.trim().toLowerCase());
}
+ /**
+ * Whether the value a clause reads is a secret, and so must stay out of the finding.
+ *
+ * <p>A finding's message ends up in CI build logs, JSON and SARIF reports and the GUI. DB-001
+ * used to report a hardcoded database password as "(current value: secret123)", decrypting an
+ * {@code Encrypted} one on the way, so the rule meant to catch exposed passwords exposed them.
+ * {@code NO_HARDCODED} only ever looks at secrets; any other condition on a field named like one
+ * is treated the same, with the default name patterns as well as the rule's own.
+ */
+ private static boolean holdsASecret(
+ CustomLintRule rule, RuleCondition condition, String fieldName) {
+ if (condition == RuleCondition.NO_HARDCODED) {
+ return true;
+ }
+ if (Utils.isEmpty(fieldName)) {
+ return false;
+ }
+ String name = fieldName.toLowerCase();
+ for (List<String> patterns :
+ List.of(DEFAULT_SECRET_FIELD_PATTERNS, getPasswordFieldPatterns(rule))) {
+ for (String pattern : patterns) {
+ if (!Utils.isEmpty(pattern) && name.endsWith(pattern.trim().toLowerCase())) {
Review Comment:
**[bug]** A non-`NO_HARDCODED` value is redacted only when the rule's field string ends with one of these camelCase suffixes. That is not the same set of fields Hop stores as passwords. `@HopMetadataProperty(password = true)` names that do not end with the list still go through `generateErrorMessage` and are appended as `(current value: ...)`. Concrete cases: `accessKey` (S3 and Minio metadata), `keyPassphrase`, `privateKeyPassphrase`, `authorizationHeaderValue` / `auth_header_value`, and `oauthJwtPrivateKey` / `oauth_jwt_private_key`. The same `endsWith` check misses the serialized keys rule authors copy from metadata JSON when the pattern is a compound word: `secret_key` does not end with `secretkey`, nor do `api_key` and `secret_access_key`. `extractFieldFromObject` documents that serialized name as the one a rule uses, while `namesASecret` only ever sees the Java field name, so this gap is new. A `MATCHES_PATTERN` rule of `^\\$\\{.*\\}$` on `accessKey` — the same shape as th
e new composed-password test — still prints the key. `NOT_EMPTY` does not, because it only fires when the value is empty.
**Suggestion:** Treat the field that was actually read as secret when its `@HopMetadataProperty.password()` is true, and compare suffixes after stripping `_` so `secret_key` matches `secretKey`. Cover `accessKey` and `oauth_jwt_private_key` with a `MATCHES_PATTERN` test that asserts the value is absent.
##########
plugins/misc/lint/src/main/java/org/apache/hop/lint/CustomRuleExecutor.java:
##########
@@ -940,23 +952,54 @@ private static boolean namesASecret(Field field, String pattern) {
return field.getName().toLowerCase().endsWith(pattern.trim().toLowerCase());
}
+ /**
+ * Whether the value a clause reads is a secret, and so must stay out of the finding.
+ *
+ * <p>A finding's message ends up in CI build logs, JSON and SARIF reports and the GUI. DB-001
+ * used to report a hardcoded database password as "(current value: secret123)", decrypting an
+ * {@code Encrypted} one on the way, so the rule meant to catch exposed passwords exposed them.
+ * {@code NO_HARDCODED} only ever looks at secrets; any other condition on a field named like one
+ * is treated the same, with the default name patterns as well as the rule's own.
+ */
+ private static boolean holdsASecret(
+ CustomLintRule rule, RuleCondition condition, String fieldName) {
+ if (condition == RuleCondition.NO_HARDCODED) {
Review Comment:
**[suggestion]** Every `NO_HARDCODED` clause is treated as a secret, so the current value is dropped even when the field is not one. `RuleCondition.NO_HARDCODED` is "Field must use variables, not hardcoded values", and `RuleTargetFields.getCompatibleConditions` offers it for every string field. A custom hostname, URL, or SQL rule that used this condition previously reported `(current value: ...)`, which is what makes the finding actionable, and now omits it. The comment that this condition "only ever looks at secrets" does not match the enum or the rule builder; the core pack happens to use it only for passwords, but that is not the condition's contract. DB-001 stays redacted either way, because its field is `password`.
**Suggestion:** Remove this unconditional branch and decide from the field (name suffix and `password = true`). Keep a test that a non-secret `NO_HARDCODED` rule still includes the current value.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@hop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fixes #8730 : Keep the values of password fields out of lint findings (hop)
Posted by "bamaer (via GitHub)" <gi...@apache.org>.
bamaer commented on PR #8737:
URL: https://github.com/apache/hop/pull/8737#issuecomment-6021314611
Both points addressed in the latest commit.
**[bug] Secrets not named like a secret.** Whether a value is hidden now depends on the field that was read. It is hidden when that field has `@HopMetadataProperty(password = true)` (resolved through the same lookup as the value, so the serialized key and the Java name both work, on metadata, transforms, actions and dotted paths), or when its name ends with one of the secret patterns. Name matching ignores `_`, so `secret_key`, `api_key` and `secret_access_key` match.
**[suggestion] `NO_HARDCODED`.** The unconditional branch is removed. A `NO_HARDCODED` rule on a field that is not a secret reports `(current value: ...)` again. DB-001 stays hidden because its field is `password`.
**Tests** (`SecretValueInFindingTest`):
- `MATCHES_PATTERN` on `accessKey` and on `oauth_jwt_private_key`: value absent
- `secret_key` key with no password flag: value absent
- composed rule: `actual: hidden` for the `accessKey` clause
- non-secret `NO_HARDCODED` rule: still reports the current value
- transform (by Java name and by key) and action with a `password = true` field: value absent
The new tests fail on the previous commit.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@hop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fixes #8730 : Keep the values of password fields out of lint findings (hop)
Posted by "mattcasters (via GitHub)" <gi...@apache.org>.
mattcasters merged PR #8737:
URL: https://github.com/apache/hop/pull/8737
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@hop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org