ddreyer commented on code in PR #19884:
URL: https://github.com/apache/pinot/pull/19884#discussion_r4288996228
##########
pinot-spi/src/main/java/org/apache/pinot/spi/utils/Obfuscator.java:
##########
@@ -58,16 +58,25 @@
public final class Obfuscator {
private static final String DEFAULT_MASKED_VALUE = "*****";
+ /// Suffix-anchored, so that a key is masked for what it ends in and a
structural name such as
+ /// `pinot.broker.auth.provider.class` stays readable. Beyond the generic
credential words, this covers
+ /// credentials whose key does not end in one: the AWS `accessKey` /
`accessKeyId` pair (as in
+ /// `input.fs.prop.accessKey` and `fs.s3a.access.key`), Kafka's
`sasl.jaas.config`, Confluent's
+ /// `basic.auth.user.info` (a literal `user:password`), and GCP
service-account key material.
private static final List<Pattern> DEFAULT_PATTERNS =
Stream.of("(?i).*secret$", "(?i).*secret[\\s_-]*key$",
"(?i).*api[\\s_-]*key$", "(?i).*password$",
- "(?i).*keytab$", "(?i).*token$")
+ "(?i).*passwd$", "(?i).*passphrase$", "(?i).*keytab$",
"(?i).*token$", "(?i).*credentials?$",
+ "(?i).*access[\\s._-]*key([\\s._-]*id)?$",
"(?i).*private[\\s._-]*key([\\s._-]*id)?$",
Review Comment:
Good catch, thanks. Fixed in 4459e3113: the secret-key and api-key patterns
now allow a dot as well, so `fs.s3a.secret.key` is masked, and
`fs.s3a.secret.key` / `secret.key` are in the masked-keys test. While checking
this I found `fs.azure.account.key` had the same problem, so there is now an
`account.key` pattern too.
##########
pinot-spi/src/main/java/org/apache/pinot/spi/utils/Obfuscator.java:
##########
@@ -58,16 +58,25 @@
public final class Obfuscator {
private static final String DEFAULT_MASKED_VALUE = "*****";
+ /// Suffix-anchored, so that a key is masked for what it ends in and a
structural name such as
+ /// `pinot.broker.auth.provider.class` stays readable. Beyond the generic
credential words, this covers
+ /// credentials whose key does not end in one: the AWS `accessKey` /
`accessKeyId` pair (as in
+ /// `input.fs.prop.accessKey` and `fs.s3a.access.key`), Kafka's
`sasl.jaas.config`, Confluent's
+ /// `basic.auth.user.info` (a literal `user:password`), and GCP
service-account key material.
private static final List<Pattern> DEFAULT_PATTERNS =
Stream.of("(?i).*secret$", "(?i).*secret[\\s_-]*key$",
"(?i).*api[\\s_-]*key$", "(?i).*password$",
- "(?i).*keytab$", "(?i).*token$")
+ "(?i).*passwd$", "(?i).*passphrase$", "(?i).*keytab$",
"(?i).*token$", "(?i).*credentials?$",
+ "(?i).*access[\\s._-]*key([\\s._-]*id)?$",
"(?i).*private[\\s._-]*key([\\s._-]*id)?$",
+ "(?i).*jaas[\\s._-]*config$", "(?i).*user[\\s._-]*info$",
"(?i).*json[\\s._-]*key$",
+ "(?i).*key[\\s._-]*file$",
"(?i).*key[\\s._-]*content([\\s._-]*base64)?$")
Review Comment:
Agreed, those values are paths and not secrets, and the GCS service-account
keyfile I was aiming at is a path as well. I dropped the key-file pattern in
4459e3113 and added `sslKeyFile`, `publicKeyFile` and the GCS keyfile to the
stays-readable test. Inline key material (`keyContentBase64`, `jsonKey`,
`privateKey`) is still masked.
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
