atiaomar1978-hub commented on PR #25113: URL: https://github.com/apache/camel/pull/25113#issuecomment-5084307651
Thanks for the review, @davsclaus — the latest push (`77607275443`) addresses your feedback and follow-ups from automated review (Bugbot + Grok). _AI-generated comment on behalf of @atiaomar1978-hub._ --- ## @davsclaus review ### 1. Fast pre-checks before regex `SensitiveUtils.maskUserInfoCredentials`, `maskPemPrivateKeyBlocks`, and the call path from `DefaultMaskingFormatter.format()` now skip the regex engine when markers are absent: - URI userinfo: return early when the text does not contain `://` - PEM private keys: return early when the text does not contain `-----BEGIN` (case-insensitive via `StringHelper.containsIgnoreCase`, consistent with the `CASE_INSENSITIVE` PEM pattern) Plain log lines such as `Hello World` no longer pay regex cost. Covered by `maskValueShapeHelpersSkipRegexWhenMarkersAbsent` and `formatPlainTextUnchanged`. ### 2. Duplicate `USERINFO_PASSWORD` pattern name The log-masking pattern in `SensitiveUtils` is renamed to **`URI_USERINFO_PASSWORD_IN_TEXT`**, with Javadoc explaining how it differs from `URISupport`'s `USERINFO_PASSWORD` used in `sanitizeUri()` (stricter scheme/userinfo rules for free text vs. URI sanitization). The regex prefix is **non-greedy** (`*?:`) so passwords containing `:` are fully masked (e.g. `http://user:pass:word@host` → password replaced in one shot), aligned with `URISupport`'s non-greedy userinfo handling. ### 3. Empty-keyword formatter behavior Previously, `format()` with an empty keyword set skipped all masking. Value-shape masking (URI userinfo + PEM) now **always** runs after name-based passes, which is intentional so embedded credentials in connection strings are still redacted when no sensitive key names appear. Documented in: - `DefaultMaskingFormatter` class Javadoc - Camel 4.22 upgrade guide (`camel-support - DefaultMaskingFormatter masks URI userinfo and PEM private keys`) Test: `formatMasksValueShapesWhenKeywordsEmpty` (URI/PEM still masked; bare `password=visible` unchanged without keywords). --- ## Bugbot | Pass | Finding | Resolution | |------|---------|------------| | Pre-fix | PEM fast path was case-sensitive while the regex is `CASE_INSENSITIVE` → lowercase `-----begin ...` could skip masking | PEM pre-check uses `StringHelper.containsIgnoreCase(source, "-----BEGIN")`; test added for lowercase PEM headers | | Post-fix (branch vs `main`) | — | **No bugs reported** | --- ## Grok (second review) | Severity | Topic | Resolution | |----------|--------|------------| | Medium | Colon in password only partly masked (greedy userinfo prefix) | Non-greedy `[^/@\\s\"']*?:` + test `http://user:pass:word@host` | | Medium | PEM fast path vs case-insensitive regex mismatch | Same as Bugbot fix above | | Low | Orphaned duplicate Javadoc on the pattern field | Merged into a single block | | Low | Missing tests for colon-in-password / lowercase PEM | Added in `SensitiveUtilsTest` | Items already in good shape per Grok: empty-username Redis (`redis://:pass@`), `mailto:`, user-only userinfo, naming/docs distinction from `URISupport`, empty-keywords + upgrade guide. --- ## Tests (local) ```text JAVA_HOME=/opt/homebrew/opt/openjdk@21 \ ./mvnw -pl core/camel-util,core/camel-core -am test \ -Dtest=SensitiveUtilsTest,DefaultMaskingFormatterTest ``` All tests passed locally; awaiting CI. --- Happy to adjust further if anything should stay a strict no-op (e.g. empty keywords) or if we should share/reconcile regex with `URISupport` more directly in a follow-up. -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
