zanarellidev opened a new pull request, #7639: URL: https://github.com/apache/kyuubi/pull/7639
Follow-up from a private report I sent to [email protected] (forwarded to the Kyuubi PMC, cc @Yikf @pan3793 @turboFei). @aajisaka reviewed the original patch (which made the REST session-conf response fail closed on an unset regex) and flagged it as a breaking change for the conf-map shape, and suggested the master-branch fix should instead give the redaction regex a real default. This PR is that. ### Why are the changes needed? `kyuubi.server.conf.retrieveMode` defaults to `REDACTED` since 1.12.0, but the regex it depends on (`kyuubi.server.redaction.regex`) has no default (`createOptional`, no `createWithDefault`). `ApiUtils.buildConf`'s `REDACTED` case hands that `None` straight to `Utils.redact`, whose `None` branch returns the conf map untouched. So `GET /api/v1/sessions` and `GET /api/v1/sessions/{handle}` return session conf — including `spark.password`-style keys — unredacted on a stock install, even though the retrieve mode is `REDACTED`. `SessionsResourceSuite` masked this: its shared `conf` sets `kyuubi.server.redaction.regex` to `(?i)password` explicitly (line 45), so no existing test exercises the stock-default path. ### Fix Give `kyuubi.server.redaction.regex` a default of `"(?i)secret|password|token|access[.]key"` — the same fallback pattern `SparkSQLEngine` already uses for `spark.redaction.regex` (`SparkSQLEngine.scala:275`), so this isn't a new convention. `REDACTED` now actually redacts by default instead of silently behaving like `ORIGINAL`. As a side effect, `Utils.redactCommandLineArgs` (which shares the same config entry for redacting spawned command-line args) gets the same improvement. Also updated `SERVER_CONF_RETRIEVE_MODE`'s doc to reflect that the redaction pattern now has a default. ### How was this patch tested? - Added `SessionsResourceSuite`: "get /sessions redacts spark confs by default with no explicit redaction regex" — explicitly unsets the redaction pattern (`withNoExplicitRedactionPattern`) and asserts a `spark.password` value comes back redacted, not raw. - Verified fail-before/pass-after locally: with the config default reverted, the new test fails with the raw secret value present in the response (`"shouldNeverLeak" equaled "shouldNeverLeak"`); with the default applied, it passes. - `build/mvn test -pl kyuubi-server -am -DwildcardSuites=org.apache.kyuubi.server.api.v1.SessionsResourceSuite`: 9/9 relevant tests pass (3 unrelated tests in this suite — `submit operation and get operation handle`, `fix kyuubi session leak caused by engine stop`, `list all type operations under session` — fail identically on unmodified `master` in this environment; confirmed pre-existing/environmental, not related to this change). - `dev/reformat` run; diff scoped back down to the 4 files this PR touches (the initial pass also reformatted unrelated pre-existing drift across `kyuubi-hive-beeline`/`kyuubi-hive-jdbc`, reverted before committing). ### Was this patch authored or co-authored using generative AI tooling? Assisted-by: Claude Sonnet 5 -- 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]
