You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@kyuubi.apache.org by "zanarellidev (via GitHub)" <gi...@apache.org> on 2026/08/20 01:42:58 UTC
[PR] Give kyuubi.server.redaction.regex a default so REDACTED redacts on a stock install [kyuubi]
zanarellidev opened a new pull request, #7639:
URL: https://github.com/apache/kyuubi/pull/7639
Follow-up from a private report I sent to security@apache.org (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: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
Re: [PR] Give kyuubi.server.redaction.regex a default so REDACTED redacts on a stock install [kyuubi]
Posted by "pan3793 (via GitHub)" <gi...@apache.org>.
pan3793 closed pull request #7639: Give kyuubi.server.redaction.regex a default so REDACTED redacts on a stock install
URL: https://github.com/apache/kyuubi/pull/7639
--
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: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
Re: [PR] Give kyuubi.server.redaction.regex a default so REDACTED redacts on a stock install [kyuubi]
Posted by "pan3793 (via GitHub)" <gi...@apache.org>.
pan3793 commented on PR #7639:
URL: https://github.com/apache/kyuubi/pull/7639#issuecomment-5350587796
@zanarellidev, thanks for raising this PR.
it's unnecessary to emphasize the default value, because the generated config docs have a "default value" column, and when you touch configs (e.g., introduce new configs or change docs/default values, call `dev/gen/gen_all_config_docs.sh` to regenerate the config docs). and for this change, you should update `docs/deployment/migration-guide.md`.
--
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: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
Re: [PR] Give kyuubi.server.redaction.regex a default so REDACTED redacts on a stock install [kyuubi]
Posted by "zanarellidev (via GitHub)" <gi...@apache.org>.
zanarellidev commented on PR #7639:
URL: https://github.com/apache/kyuubi/pull/7639#issuecomment-5360870232
Done, pushed e01fbd846130fc1a6d9147104f3cd1cba9960aa4. Reverted both doc strings to the pre-PR wording and regenerated docs/configuration/settings.md via dev/gen/gen_all_config_docs.sh.
That regen actually caught a real bug: the new default `(?i)secret|password|token|access[.]key` is the first regex default with a literal `|` in it, and AllKyuubiConfiguration's table builder joins cells with `|` and doesn't escape it, so the row split into bogus columns. Fixed that too (escaping the Default column only, since the Meaning column already leans on backtick code spans to protect any `|` it has). Added the migration-guide.md entry as well.
--
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: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
Re: [PR] Give kyuubi.server.redaction.regex a default so REDACTED redacts on a stock install [kyuubi]
Posted by "pan3793 (via GitHub)" <gi...@apache.org>.
pan3793 commented on PR #7639:
URL: https://github.com/apache/kyuubi/pull/7639#issuecomment-5367314988
**Merge Summary:**
- merged into master https://github.com/apache/kyuubi/commit/3bc223dbec2ace1692ba22769404c9dd329981ed
*Posted by `merge_kyuubi_pr.py`*
--
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: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org