Skip to content

[HOTFIX] Validate note access for interpreter bindings - #5431

Open
jongyoul wants to merge 1 commit into
apache:masterfrom
jongyoul:codex/security-interpreter-bindings-authz
Open

[HOTFIX] Validate note access for interpreter bindings#5431
jongyoul wants to merge 1 commit into
apache:masterfrom
jongyoul:codex/security-interpreter-bindings-authz

Conversation

@jongyoul

Copy link
Copy Markdown
Member

What is this PR for?

Ensure that interpreter binding operations follow the permissions of the associated note.

  • Reading interpreter bindings requires reader permission.
  • Updating interpreter bindings requires writer permission.
  • Requests without the required permission stop before returning binding data or applying changes.
  • Add regression tests for permitted and rejected read/write requests.

What type of PR is it?

Hot Fix

Todos

  • Add note permission checks for interpreter binding operations
  • Add regression tests
  • Run NotebookServerTest

What is the Jira issue?

N/A

How should this be tested?

mkdir -p spark/interpreter/target
./mvnw -pl spark/interpreter resources:resources@copy-interpreter-setting
./mvnw -pl zeppelin-server -Dtest=NotebookServerTest test

Result: 25 tests run, 0 failures, 0 errors.

Screenshots (if appropriate)

N/A

Questions:

  • Does the license files need to update? No.
  • Is there breaking changes for older versions? No API compatibility changes. Requests without the required note permission are now rejected as intended.
  • Does this needs documentation? No.

@jongyoul
jongyoul marked this pull request as ready for review August 19, 2026 11:09
Copilot AI lite review requested due to automatic review settings August 19, 2026 11:09

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Warning

Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.

This hotfix enforces note-level permissions for interpreter binding operations so that binding reads/writes are rejected unless the caller has the appropriate note access.

Changes:

  • Add reader permission check to GET_INTERPRETER_BINDINGS.
  • Add writer permission check to SAVE_INTERPRETER_BINDINGS and suppress successful response when unauthorized.
  • Add regression tests covering permitted and rejected read/write requests.

Reviewed changes

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

File Description
zeppelin-server/src/main/java/org/apache/zeppelin/socket/NotebookServer.java Adds authz gates for reading/writing interpreter bindings and conditions response on authorization.
zeppelin-server/src/test/java/org/apache/zeppelin/socket/NotebookServerTest.java Adds regression tests verifying authz enforcement for interpreter binding read/write operations.
Suppressed comments (1)

zeppelin-server/src/main/java/org/apache/zeppelin/socket/NotebookServer.java:1

  • permitted is returned as true even when note == null (line 649), which makes the method send an OP.INTERPRETER_BINDINGS response for a non-existent note. If permitted is intended to reflect “authorized & actionable”, return false when note == null (and consider initializing permitted to false and setting it to true only after both the permission check passes and the note exists). This keeps the response behavior consistent with the intention of “stop before returning binding data or applying changes.”
/*

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

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.

2 participants