allthingssecurity commented on code in PR #26997:
URL: https://github.com/apache/camel/pull/26997#discussion_r4129055767
##########
core/camel-base/src/main/java/org/apache/camel/component/properties/DefaultPropertiesParser.java:
##########
@@ -369,7 +373,7 @@ private String getPropertyValue(String prevKey, String key,
String input) {
} else {
if (log.isDebugEnabled()) {
log.debug("Property with key [{}] applied by
function [{}] -> {}", key, function.getName(),
- value);
+ function.isSensitive() ? MASK : mask(key,
value));
Review Comment:
For a function that is not sensitive, `mask(key, value)` gets the whole key
including the function prefix. `SensitiveUtils.containsSensitive` then sees
`env:DB_PASSWORD`, which has no `.` to split on, and `env:db_password` is not
in the list. So `{{env:DB_PASSWORD}}`, a common way to pass a secret into a
container, is still logged in clear. That happens at DEBUG here and at TRACE in
`PropertiesComponent.parseUri`, whose `isSensitiveKey` cuts at the first `:`
and so checks `env`. I checked it on this branch with `{{sys:DB_PASSWORD}}`,
which takes the same path because `sys` is also a non-sensitive function:
```
Property with key [sys:DB_PASSWORD] applied by function [sys] -> Secret-sys
Parsed uri {{sys:DB_PASSWORD}} -> Secret-sys
```
(`{{sys:db.password}}` is masked, because `containsSensitive` keeps only the
part after the last `.`.) Checking the part after the function prefix
(`DB_PASSWORD`) in both places would cover it.
_Review by Claude Code on behalf of allthingssecurity_
##########
core/camel-base/src/main/java/org/apache/camel/component/properties/PropertiesComponent.java:
##########
@@ -346,7 +380,9 @@ protected String parseUri(final String uri,
PropertiesLookup properties, boolean
// Remove the escape characters if any
answer = unescape(answer);
}
- LOG.trace("Parsed uri {} -> {}", uri, answer);
+ if (LOG.isTraceEnabled()) {
+ LOG.trace("Parsed uri {} -> {}", uri, isSensitive(uri) ? "xxxxxx"
: answer);
Review Comment:
`isSensitive(uri)` only looks at the text of the uri, so a vault value
reached through an ordinary property is still logged. With
`app.db.conn={{myvault:db/conn}}` (a function whose `isSensitive()` is true),
resolving `{{app.db.conn}}` on this branch logs at TRACE:
```
Parsed uri {{app.db.conn}} -> Vault-db/conn
```
The function's own line is masked; only this summary line leaks. One option
is to let the parser record that a sensitive function or key was used while
resolving the uri, and mask on that, instead of re-reading the uri text.
Related, possibly as a follow-up since it is not logging: the same case
shows the secret in the properties dev console.
`getResolvedValue("app.db.conn")` holds `value=Vault-db/conn, source=myvault`,
and `PropertiesDevConsole.toPropertyEntry` masks only when the key is
sensitive. With `PropertiesFunction.isSensitive()` now available, the console
could also mask when `source` is a sensitive function.
_Review by Claude Code on behalf of allthingssecurity_
--
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]
