Skip to content

[MINOR] Drop ticket value from WebSocket debug log statements#5228

Merged
jongyoul merged 1 commit intoapache:masterfrom
jongyoul:ZEPPELIN-ws-ticket-log-redact
May 10, 2026
Merged

[MINOR] Drop ticket value from WebSocket debug log statements#5228
jongyoul merged 1 commit intoapache:masterfrom
jongyoul:ZEPPELIN-ws-ticket-log-redact

Conversation

@jongyoul
Copy link
Copy Markdown
Member

@jongyoul jongyoul commented May 6, 2026

What is this PR for?

Removes the WebSocket auth ticket value from three LOGGER.debug call sites in NotebookServer.onMessage. The ticket is a per-session UUID and adds no debugging value beyond the principal that owns it; emitting the raw value makes it visible to anyone with access to log files or downstream log collectors.

The three call sites and the change applied to each:

  • RECEIVE block — drops the RECEIVE TICKET column. The remaining op / principal / roles / data columns are sufficient to identify the message.
  • "no ticket on file" branch — logs the principal that has no entry instead of echoing back the rejected ticket.
  • "ticket mismatch" branch — logs the principal whose ticket did not match, rather than both raw values.

Message.toString() does not include the ticket field, so the surrounding LOGGER.trace("RECEIVE MSG = " + receivedMessage) already does not leak it.

What type of PR is it?

Improvement

What is the Jira issue?

N/A — minor logging hygiene change, no behavioral or API change.

How should this be tested?

Diff is self-evident. The three changed sites stay on the existing branches; behavior (what is returned to the client, what is rejected) is unchanged. Existing NotebookServerTest continues to exercise these paths.

Screenshots (if appropriate)

N/A

Questions:

  • Does the license files need to update? No
  • Is there breaking changes for older versions? No
  • Does this needs documentation? No

The WebSocket auth ticket is a per-session UUID and exposes nothing
useful for debugging beyond the principal that owns it. Logging the
raw value at DEBUG level makes the value visible to anyone who reads
log files or whichever observability stack the logs flow into.

Replace the three call sites in NotebookServer.onMessage that emit
the ticket value with messages keyed on the principal:

- RECEIVE block: drop the "RECEIVE TICKET" column entirely. The
  remaining op / principal / roles / data fields keep the message
  identifiable.
- "no ticket on file" branch: log the principal that has no entry
  rather than the rejected ticket.
- ticket-mismatch branch: log the principal whose ticket did not
  match rather than both raw values.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@jongyoul jongyoul marked this pull request as ready for review May 9, 2026 11:36
Copilot AI review requested due to automatic review settings May 9, 2026 11:36
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR improves WebSocket logging hygiene in NotebookServer.onMessage by removing the per-session auth ticket value from debug logs, reducing the risk of exposing sensitive session identifiers via log files/collectors without changing runtime behavior.

Changes:

  • Removed the ticket value from the general “RECEIVE” debug log line.
  • Updated “no ticket on file” and “ticket mismatch” debug logs to reference the principal instead of echoing ticket values.

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

Copy link
Copy Markdown
Member

@ParkGyeongTae ParkGyeongTae left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Sensible logging hygiene fix — the ticket is a session credential and has no business sitting in debug logs. Keeping principal is enough to identify the message. No behavior change, existing tests cover the paths.

@jongyoul jongyoul merged commit e1e59bc into apache:master May 10, 2026
42 of 47 checks passed
jongyoul added a commit that referenced this pull request May 10, 2026
### What is this PR for?

Removes the WebSocket auth ticket value from three `LOGGER.debug` call sites in `NotebookServer.onMessage`. The ticket is a per-session UUID and adds no debugging value beyond the principal that owns it; emitting the raw value makes it visible to anyone with access to log files or downstream log collectors.

The three call sites and the change applied to each:

- **RECEIVE block** — drops the `RECEIVE TICKET` column. The remaining `op` / `principal` / `roles` / `data` columns are sufficient to identify the message.
- **"no ticket on file" branch** — logs the principal that has no entry instead of echoing back the rejected ticket.
- **"ticket mismatch" branch** — logs the principal whose ticket did not match, rather than both raw values.

`Message.toString()` does not include the ticket field, so the surrounding `LOGGER.trace("RECEIVE MSG = " + receivedMessage)` already does not leak it.

### What type of PR is it?

Improvement

### What is the Jira issue?

N/A — minor logging hygiene change, no behavioral or API change.

### How should this be tested?

Diff is self-evident. The three changed sites stay on the existing branches; behavior (what is returned to the client, what is rejected) is unchanged. Existing `NotebookServerTest` continues to exercise these paths.

### Screenshots (if appropriate)

N/A

### Questions:

- Does the license files need to update? No
- Is there breaking changes for older versions? No
- Does this needs documentation? No

Closes #5228 from jongyoul/ZEPPELIN-ws-ticket-log-redact.

Signed-off-by: Jongyoul Lee <jongyoul@gmail.com>
(cherry picked from commit e1e59bc)
Signed-off-by: Jongyoul Lee <jongyoul@gmail.com>
@jongyoul
Copy link
Copy Markdown
Member Author

Merged into master (e1e59bc).
Cherry-picked into branch-0.12.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants