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

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

Why are the changes needed?

This PR addresses below issue with the Kyuubi sessions API:

1Incorrect 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.

How was this patch tested?

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.

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

+      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

  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:03Z

+    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

    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

-    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

+    } 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

…AuthenticationFilter identity for list session.

turboFei · 2026-03-27T04:32:24Z

      mediaType = MediaType.APPLICATION_JSON,
      array = new ArraySchema(schema = new Schema(implementation = classOf[SessionData])))),
-    description = "get the list of all live sessions")
+    description = "get the list of live sessions for the current user")


how about only fix the 404 issue in this PR?

sure, I will only include 404 fix and revert others

…sions ### Why are the changes needed? This PR addresses below issue with the Kyuubi sessions API: 1**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. ### How was this patch tested? **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. ### Was this patch authored or co-authored using generative AI tooling? No Closes #7319 from Yutong-1424/hotfix/yutong/fix_kyuubi_sessions. Closes #7319 e2e3fd8 [Han, Yutong] revert security improvement changes 0b0d0e7 [Han, Yutong] Replace admin with clientPrincipalUser so openSession’s user matches AuthenticationFilter identity for list session. aade31c [Han, Yutong] fix: compare session identifier as string in SessionsResourceSuite 14c03d9 [Han, Yutong] Replace try/catch exception handling with direct session existence check 79f6406 [Han, Yutong] - Replace try/catch with getSessionOption() for cleaner 404 handling - Fix type mismatch - Update test assertions for correct redaction verification - Configure redaction pattern at suite level a4d81e9 [Han, Yutong] Fix kyuubi sessions API to return 404 for non-existent sessions and add security improvements Authored-by: Han, Yutong <yutonghan@geico.com> Signed-off-by: Fei Wang <fwang12@ebay.com> (cherry picked from commit 1bc129a) Signed-off-by: Fei Wang <fwang12@ebay.com>

turboFei · 2026-03-31T17:19:02Z

thanks, merged to master, branch-1.11 and branch-1.10(1.10.4).

Yutong-1424 · 2026-04-05T05:51:32Z

Thanks! I would like to make another pr to the part that make Kyuubi sessions only return sessions belong to the calling user and redact credential data, and I would like to know if any concerns for this change

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 3 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

Replace admin with clientPrincipalUser so openSession’s user matches …

0b0d0e7

…AuthenticationFilter identity for list session.

turboFei reviewed Mar 27, 2026

View reviewed changes

revert security improvement changes

e2e3fd8

turboFei approved these changes Mar 31, 2026

View reviewed changes

turboFei changed the title ~~Fix kyuubi sessions to return 404 for non-existent sessions and add security improvements~~ Fix kyuubi sessions to return 404 for non-existent sessions Mar 31, 2026

turboFei assigned Yutong-1424 Mar 31, 2026

turboFei added this to the v1.10.4 milestone Mar 31, 2026

turboFei closed this in 1bc129a Mar 31, 2026

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

Conversation

Yutong-1424 commented Jan 30, 2026 • edited by turboFei Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

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!

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!

turboFei Mar 27, 2026

Choose a reason for hiding this comment

Uh oh!

Yutong-1424 Mar 27, 2026

Choose a reason for hiding this comment

Uh oh!

turboFei commented Mar 31, 2026

Uh oh!

Yutong-1424 commented Apr 5, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Yutong-1424 commented Jan 30, 2026 •

edited by turboFei

Loading