Fix kyuubi sessions to return 404 for non-existent sessions and add security improvements by Yutong-1424 · Pull Request #7319 · apache/kyuubi

Yutong-1424 · 2026-01-30T17:08:36Z

Why are the changes needed?

This PR addresses two important issues with the Kyuubi sessions API:

Incorrect HTTP status codes: When attempting to delete a non-existent session, the API currently returns HTTP 500 (Internal Server Error), which is misleading. A non-existent resource should return HTTP 404 (Not Found) to properly indicate the resource doesn't exist.
Security and privacy concerns:
- The /api/v1/sessions endpoint returns all sessions for all users, potentially exposing sensitive information about other users' sessions
- Session configuration data may contain sensitive credentials (e.g., passwords, tokens) that should be redacted before being returned to clients

How was this patch tested?

Unit tests were added to verify both changes:

Test for 404 response on delete non-existent session: Added delete_non-existent_admin_session_returns_404 test in AdminResourceSuite.scala that verifies deleting a non-existent session returns HTTP 404 instead of 500.
Test for config redaction: Added get /sessions returns redacted spark confs test in SessionsResourceSuite.scala that:
- Creates a session with a sensitive configuration value (spark.password)
- Retrieves the session list via GET /api/v1/sessions
- Verifies that the sensitive value is redacted (replaced with asterisks) rather than returned in plain text

Was this patch authored or co-authored using generative AI tooling?

No

…dd security improvements

Yutong-1424 · 2026-03-05T16:44:23Z

Hi @turboFei, could you help to review this pull request

turboFei · 2026-03-06T04:50:32Z

kyuubi-server/src/main/scala/org/apache/kyuubi/server/api/v1/AdminResource.scala

+      Response.ok(s"Session $sessionHandleStr is closed successfully.").build()
+    } catch {
+      case e: org.apache.kyuubi.KyuubiSQLException
+          if e.getMessage != null && e.getMessage.startsWith("Invalid ") =>


The implementation is tricky.
Maybe you can check session exists directly,
fe.be.sessionManager.getSessionOption()

Thanks for the review @turboFei ! That's good point, using getSessionOption() is much cleaner than catching the exception and matching on the message string. I'll update both AdminResource and SessionsResource to check session existence directly before calling closeSession. Will push the fix shortly.

Copilot

Pull request overview

This PR addresses two issues in the Kyuubi sessions REST API: (1) returning HTTP 404 instead of 500 when deleting a non-existent session, and (2) privacy/security improvements — filtering /api/v1/sessions to only show the current user's sessions and redacting sensitive config values from API responses using the existing kyuubi.server.redaction.regex mechanism.

Changes:

Add try/catch around closeSession calls in both SessionsResource and AdminResource to map KyuubiSQLException("Invalid ...") to HTTP 404 via NotFoundException
Add user-based filtering in SessionsResource.sessions() and redact sensitive session configuration in ApiUtils.sessionData and ApiUtils.sessionEvent using Utils.redact with SERVER_SECRET_REDACTION_PATTERN
Add unit tests verifying the 404 behavior (AdminResourceSuite) and conf redaction (SessionsResourceSuite)

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 5 comments.

Show a summary per file

File	Description
`kyuubi-server/src/main/scala/org/apache/kyuubi/server/api/ApiUtils.scala`	Adds config redaction to `sessionEvent` and `sessionData` methods; contains a type error: `Option(redactionPattern)` double-wraps an already-`Option[Regex]` value
`kyuubi-server/src/main/scala/org/apache/kyuubi/server/api/v1/SessionsResource.scala`	Adds 404 handling for `closeSession` and user-scoped filtering in `sessions()`; API description comment is now inaccurate
`kyuubi-server/src/main/scala/org/apache/kyuubi/server/api/v1/AdminResource.scala`	Adds 404 handling for admin `closeSession`; implementation looks correct
`kyuubi-server/src/test/scala/org/apache/kyuubi/server/api/v1/AdminResourceSuite.scala`	New test for 404 on deleting a non-existent session; looks correct
`kyuubi-server/src/test/scala/org/apache/kyuubi/server/api/v1/SessionsResourceSuite.scala`	New test for conf redaction; has two bugs: missing `SERVER_SECRET_REDACTION_PATTERN` config, and wrong assertion using `forall(_ == '')` vs actual replacement text `"********(redacted)"`

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-03-06T04:58:02Z

kyuubi-server/src/main/scala/org/apache/kyuubi/server/api/ApiUtils.scala

  def sessionData(session: KyuubiSession): SessionData = {
    val sessionEvent = session.getSessionEvent
+    val redactionPattern = session.sessionManager.getConf.get(SERVER_SECRET_REDACTION_PATTERN)
+    val redactedConf = Utils.redact(Option(redactionPattern), session.conf.toSeq).toMap.asJava


Same type error as in sessionEvent: Utils.redact(Option(redactionPattern), ...) passes Option[Option[Regex]] where Option[Regex] is required. The redactionPattern variable is already Option[Regex] (from conf.get(SERVER_SECRET_REDACTION_PATTERN)), so it should be passed directly without the Option() wrapping.

Copilot · 2026-03-06T04:58:02Z

kyuubi-server/src/main/scala/org/apache/kyuubi/server/api/v1/SessionsResource.scala

@@ -55,8 +55,12 @@ private[v1] class SessionsResource extends ApiRequestContext with Logging {
    description = "get the list of all live sessions")


The @ApiResponse description still says "get the list of all live sessions" but the implementation now filters sessions to only return those belonging to the currently authenticated user. The description should be updated to reflect this behavior change, e.g., "get the list of live sessions for the current user".

Suggested change

description = "get the list of all live sessions")

description = "get the list of live sessions for the current user")

Copilot · 2026-03-06T04:58:03Z

kyuubi-server/src/test/scala/org/apache/kyuubi/server/api/v1/SessionsResourceSuite.scala

+    val sessionConf = sessions.find(_.getIdentifier == sessionHandle).get.getConf
+
+    assert(sessionConf.get(sensitiveKey) != sensitiveValue)
+    assert(sessionConf.get(sensitiveKey).forall(_ == '*'))


The test asserts sessionConf.get(sensitiveKey).forall(_ == '*'), but the actual redaction replacement text in Kyuubi is "*********(redacted)" (defined as REDACTION_REPLACEMENT_TEXT in CommandLineUtils), which contains characters other than '*'. This assertion would fail even when redaction works correctly. The assertion should instead compare against REDACTION_REPLACEMENT_TEXT or at minimum check that the value is not equal to the original sensitive value and not null.

Suggested change

assert(sessionConf.get(sensitiveKey).forall(_ == '*'))

assert(sessionConf.get(sensitiveKey) != null)

Copilot · 2026-03-06T04:58:03Z

kyuubi-server/src/test/scala/org/apache/kyuubi/server/api/v1/SessionsResourceSuite.scala

    assert(sessionHandle.toString.equals(operations.head.getSessionId))
  }
+
+  test("get /sessions returns redacted spark confs") {


The test does not configure SERVER_SECRET_REDACTION_PATTERN (via conf.set(SERVER_SECRET_REDACTION_PATTERN, ...)) before asserting that the sensitive config value is redacted. Since this config entry has no default value (createOptional), the redaction mechanism will not activate, and sensitiveValue will be returned unredacted. The test needs to set the redaction pattern (e.g., "(?i)password".r) as part of the test setup or override the server config in WithKyuubiServer.

Suggested change

test("get /sessions returns redacted spark confs") {

test("get /sessions returns redacted spark confs") {

// Configure redaction so that sensitive configs like spark.password are masked

conf.set(KyuubiConf.SERVER_SECRET_REDACTION_PATTERN, "(?i)password".r)

Copilot · 2026-03-06T04:58:03Z

kyuubi-server/src/main/scala/org/apache/kyuubi/server/api/ApiUtils.scala

-    session.getSessionEvent.map(event =>
+    session.getSessionEvent.map { event =>
+      val redactionPattern = session.sessionManager.getConf.get(SERVER_SECRET_REDACTION_PATTERN)
+      val redactedConf = Utils.redact(Option(redactionPattern), event.conf.toSeq).toMap.asJava


The call Utils.redact(Option(redactionPattern), ...) has a type error. session.sessionManager.getConf.get(SERVER_SECRET_REDACTION_PATTERN) already returns Option[Regex] because OptionalConfigEntry[Regex] extends ConfigEntry[Option[Regex]]. Wrapping it in another Option(...) produces Option[Option[Regex]], which is a type mismatch with Utils.redact's first parameter type Option[Regex]. This would cause a compile-time error.

The correct call should pass redactionPattern directly (without Option()), since it's already an Option[Regex].

- Fix type mismatch - Update test assertions for correct redaction verification - Configure redaction pattern at suite level

turboFei · 2026-03-08T05:05:55Z

kyuubi-server/src/main/scala/org/apache/kyuubi/server/api/v1/SessionsResource.scala

+    } catch {
+      case e: org.apache.kyuubi.KyuubiSQLException
+          if e.getMessage != null && e.getMessage.startsWith("Invalid ") =>
+        throw new NotFoundException(


please apply same change here

Done, Thanks!

Could you please review these changes again, to see if everything is good. @turboFei

Fix kyuubi sessions API to return 404 for non-existent sessions and a…

a4d81e9

…dd security improvements

github-actions bot added the module:server label Jan 30, 2026

turboFei requested a review from Copilot March 6, 2026 04:48

Copilot started reviewing on behalf of turboFei March 6, 2026 04:49 View session

turboFei reviewed Mar 6, 2026

View reviewed changes

Copilot AI reviewed Mar 6, 2026

View reviewed changes

- Replace try/catch with getSessionOption() for cleaner 404 handling

79f6406

- Fix type mismatch - Update test assertions for correct redaction verification - Configure redaction pattern at suite level

turboFei reviewed Mar 8, 2026

View reviewed changes

yutong-27 added 2 commits March 7, 2026 22:37

Replace try/catch exception handling with direct session existence check

14c03d9

fix: compare session identifier as string in SessionsResourceSuite

aade31c

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix kyuubi sessions to return 404 for non-existent sessions and add security improvements#7319

Fix kyuubi sessions to return 404 for non-existent sessions and add security improvements#7319
Yutong-1424 wants to merge 4 commits intoapache:masterfrom
Yutong-1424:hotfix/yutong/fix_kyuubi_sessions

Yutong-1424 commented Jan 30, 2026

Uh oh!

Yutong-1424 commented Mar 5, 2026

Uh oh!

turboFei Mar 6, 2026

Uh oh!

Yutong-1424 Mar 6, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Mar 6, 2026

Uh oh!

Copilot AI Mar 6, 2026

Uh oh!

Copilot AI Mar 6, 2026

Uh oh!

Copilot AI Mar 6, 2026

Uh oh!

Copilot AI Mar 6, 2026

Uh oh!

turboFei Mar 8, 2026

Uh oh!

Yutong-1424 Mar 8, 2026

Uh oh!

Yutong-1424 Mar 18, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

		@@ -55,8 +55,12 @@ private[v1] class SessionsResource extends ApiRequestContext with Logging {
		description = "get the list of all live sessions")

	description = "get the list of all live sessions")
	description = "get the list of live sessions for the current user")

	assert(sessionConf.get(sensitiveKey).forall(_ == '*'))
	assert(sessionConf.get(sensitiveKey) != null)

Conversation

Yutong-1424 commented Jan 30, 2026

Why are the changes needed?

How was this patch tested?

Was this patch authored or co-authored using generative AI tooling?

Uh oh!

Yutong-1424 commented Mar 5, 2026

Uh oh!

turboFei Mar 6, 2026

Choose a reason for hiding this comment

Uh oh!

Yutong-1424 Mar 6, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Mar 6, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 6, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 6, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 6, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 6, 2026

Choose a reason for hiding this comment

Uh oh!

turboFei Mar 8, 2026

Choose a reason for hiding this comment

Uh oh!

Yutong-1424 Mar 8, 2026

Choose a reason for hiding this comment

Uh oh!

Yutong-1424 Mar 18, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants