You are viewing a plain text version of this content. The canonical link for it is here.
Posted to jira@kafka.apache.org by "shantanushukla (via GitHub)" <gi...@apache.org> on 2026/08/30 06:05:34 UTC

[PR] KAFKA-20964: Do not echo salt or saltedPassword in ScramParser errors [kafka]

shantanushukla opened a new pull request, #23312:
URL: https://github.com/apache/kafka/pull/23312

   JIRA: https://issues.apache.org/jira/browse/KAFKA-20964
   
   `kafka-storage format --add-scram` parses SCRAM credentials in `ScramParser`. On Base64 decode failure, `PerMechanismData` built a `FormatterException` whose message included the user-supplied `saltedpassword` (and the same for `salt`). `StorageTool` prints `getMessage()` to stderr, so credential-adjacent input was echoed in the CLI error.
   
   ### Fix
   Use a generic decode-failure message and keep the `IllegalArgumentException` as the cause. Do not concatenate the user-supplied value.
   
   ```
   Failed to decode given saltedPassword
   Failed to decode given salt
   ```
   
   This matches the existing request-side practice of not printing salt or salted password (`AlterUserScramCredentialsRequest`). Decode details remain on the cause if it is logged.
   
   ### Why existing tests missed this
   `ScramParserTest` covered valid Base64 `saltedpassword`/`salt` and missing-field errors (`name`, `password`, `salt` required with `saltedpassword`, extra keys). None of those paths throw on Base64 failure, so the decode-error message was never asserted.
   
   ### Tests
   These failed on trunk (message included the raw input) and pass after this change:
   
   - `testParsePerMechanismDataFailsWithInvalidSaltedPasswordBase64` — valid salt so construction reaches the salted-password decoder; `saltedpassword="not-valid-base64!!!"`. Trunk: `Failed to decode given saltedPassword: not-valid-base64!!!`. After: `Failed to decode given saltedPassword`.
   - `testParsePerMechanismDataFailsWithInvalidSaltBase64` — `salt="%%%"`. Trunk: `Failed to decode given salt: %%%`. After: `Failed to decode given salt`.
   
   Related existing tests still pass: full `ScramParserTest` (including valid `saltedpassword` parse and `toRecord()`).
   
   ```
   ./gradlew :metadata:test --tests org.apache.kafka.metadata.storage.ScramParserTest :metadata:checkstyleMain :metadata:checkstyleTest :metadata:spotlessCheck
   ```
   BUILD SUCCESSFUL.
   
   This work is original and is licensed to the Apache Kafka project under the Apache License, Version 2.0.


-- 
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: jira-unsubscribe@kafka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] KAFKA-20964: Do not echo salt or saltedPassword in ScramParser errors [kafka]

Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #23312:
URL: https://github.com/apache/kafka/pull/23312#issuecomment-5564569829

   A label of 'needs-attention' was automatically added to this PR in order to raise the
   attention of the committers. Once this issue has been triaged, the `triage` label
   should be removed to prevent this automation from happening again.


-- 
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: jira-unsubscribe@kafka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] KAFKA-20964: Do not echo salt or saltedPassword in ScramParser errors [kafka]

Posted by "shantanushukla (via GitHub)" <gi...@apache.org>.
shantanushukla commented on PR #23312:
URL: https://github.com/apache/kafka/pull/23312#issuecomment-6192676155

   @akhileshchg the cause assertion is in both Base64 failure tests now (`0ffc0646e0`). Each one still checks the generic message and also asserts that the `FormatterException` cause is an `IllegalArgumentException`, for salted password and for salt.


-- 
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: jira-unsubscribe@kafka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org