Skip to content

[MINOR] Tighten origin and content-type handling in REST/WebSocket layer#5229

Open
jongyoul wants to merge 5 commits into
apache:masterfrom
jongyoul:minor-cors-hardening
Open

[MINOR] Tighten origin and content-type handling in REST/WebSocket layer#5229
jongyoul wants to merge 5 commits into
apache:masterfrom
jongyoul:minor-cors-hardening

Conversation

@jongyoul
Copy link
Copy Markdown
Member

@jongyoul jongyoul commented May 6, 2026

What is this PR for?

Apply stricter defaults to the request-handling layer for tighter out-of-the-box behavior:

  • CorsFilter blocks state-changing methods (POST/PUT/DELETE/PATCH) and cross-origin preflight requests when the Origin header is not in the configured allow-list. Access-Control-Allow-Credentials is only sent when the Origin is allowed.
  • The default value of zeppelin.server.allowed.origins changes from * to empty so cross-origin browser access must be explicitly enabled. Operators relying on the previous default need to set this back to * or to specific origin(s). Same-origin / same-host and non-browser clients are unaffected.
  • A new Jersey request filter restricts REST request bodies on state-changing methods to application/json, application/x-www-form-urlencoded, or multipart/form-data; other media types are rejected with 415.
  • The default shiro.ini.template now sets cookie.sameSite = LAX.
  • ZeppelinClient.addParagraph and updateParagraph now send an explicit Content-Type: application/json header so they pass the new filter.
  • CorsUtils.isValidOrigin normalizes the Origin header to lowercase before the allow-list membership check, mirroring how the configured origins are stored, so case differences in the Origin header do not produce false rejections.
  • A small HttpMethods utility holds the shared STATE_CHANGING method set used by both the servlet filter and the Jersey filter.

What type of PR is it?

Improvement

Todos

  • CI green

Questions

  • None

Screenshots (if appropriate)

N/A

@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 tightens default security behavior in Zeppelin’s REST/WebSocket request-handling layer by restricting cross-origin state-changing requests and enforcing an allow-list of request body media types.

Changes:

  • Harden CORS behavior: block disallowed cross-origin preflights and state-changing methods; only emit Access-Control-Allow-Credentials for allowed origins; default allowed origins becomes empty.
  • Add a Jersey request filter to reject unsupported Content-Type on state-changing REST requests with bodies (415).
  • Update client and config templates to align with stricter defaults (explicit JSON Content-Type in some client calls; Shiro cookie SameSite default).

Reviewed changes

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

Show a summary per file
File Description
zeppelin-server/src/test/java/org/apache/zeppelin/server/CorsFilterTest.java Expands CORS filter tests for blocked/allowed origin and method behaviors.
zeppelin-server/src/test/java/org/apache/zeppelin/rest/filter/JsonContentTypeFilterTest.java Adds unit tests validating the new REST content-type allow-list behavior.
zeppelin-server/src/test/java/org/apache/zeppelin/conf/ZeppelinConfigurationTest.java Updates allowed-origins default expectation to “empty list”.
zeppelin-server/src/main/java/org/apache/zeppelin/utils/HttpMethods.java Introduces shared state-changing HTTP method set.
zeppelin-server/src/main/java/org/apache/zeppelin/utils/CorsUtils.java Normalizes Origin casing before allow-list membership checks.
zeppelin-server/src/main/java/org/apache/zeppelin/server/RestApiApplication.java Registers the new Jersey request filter.
zeppelin-server/src/main/java/org/apache/zeppelin/server/CorsFilter.java Tightens CORS handling (blocking disallowed cross-origin preflight/state-changing requests; conditional credentials header).
zeppelin-server/src/main/java/org/apache/zeppelin/rest/filter/JsonContentTypeFilter.java Adds filter restricting request body media types for state-changing methods.
zeppelin-server/src/main/java/org/apache/zeppelin/conf/ZeppelinConfiguration.java Changes default zeppelin.server.allowed.origins from * to empty.
zeppelin-client/src/main/java/org/apache/zeppelin/client/ZeppelinClient.java Adds explicit JSON Content-Type for two paragraph APIs.
conf/shiro.ini.template Sets default cookie.sameSite = LAX in the template.

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

Comment on lines +43 to +53
String currentHost = InetAddress.getLocalHost().getHostName().toLowerCase(Locale.ROOT);
// getAllowedOrigins() returns lowercased entries; normalize sourceHost the same way
// before the membership check so case differences in the Origin header do not produce
// false rejections of explicitly configured origins.
String normalizedOrigin =
sourceHost == null ? "" : sourceHost.toLowerCase(Locale.ROOT);

return zConf.getAllowedOrigins().contains("*")
|| currentHost.equals(sourceUriHost)
|| "localhost".equals(sourceUriHost)
|| zConf.getAllowedOrigins().contains(sourceHost);
|| zConf.getAllowedOrigins().contains(normalizedOrigin);
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

@Copilot Good catch — fixed in 1e3b2d1. ZeppelinConfiguration#getAllowedOrigins() now uses toLowerCase(Locale.ROOT) so the membership check stays consistent regardless of JVM default locale.

Comment on lines +58 to +70
if (sourceHost != null && !sourceHost.isEmpty()) {
try {
if (CorsUtils.isValidOrigin(sourceHost, zConf)) {
allowedOrigin = sourceHost;
}
} catch (URISyntaxException e) {
LOGGER.warn("Rejecting request with malformed Origin header: {}", sourceHost);
}
} catch (URISyntaxException e) {
LOGGER.error("Exception in WebDriverManager while getWebDriver ", e);
}

if (((HttpServletRequest) request).getMethod().equals("OPTIONS")) {
HttpServletResponse resp = ((HttpServletResponse) response);
addCorsHeaders(resp, origin);
return;
if (allowedOrigin.isEmpty() && (isCorsPreflight(httpRequest) || isStateChanging(method))) {
LOGGER.warn("Blocking cross-origin {} request from disallowed Origin: {}",
method, sourceHost);
httpResponse.sendError(HttpServletResponse.SC_FORBIDDEN, "Origin not allowed");
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

@Copilot Fixed in 1e3b2d1. CorsFilter now also catches UnknownHostException and treats the origin as not allowed instead of letting the exception surface as a 500.

Comment on lines 593 to 597
HttpResponse<JsonNode> response = Unirest.post("/notebook/{noteId}/paragraph")
.routeParam("noteId", noteId)
.header("Content-Type", "application/json")
.body(bodyObject.toString())
.asJson();
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

@Copilot Good catch — addressed in 8607616 and 396b3fd. All remaining POST/PUT calls in ZeppelinClient that send a body now explicitly set Content-Type: application/json.

Comment on lines +30 to +42
/**
* Restricts the request body media types accepted by REST endpoints to a small allow-list.
* Requests carrying state-changing methods (POST/PUT/DELETE/PATCH) with a body must use
* {@code application/json}, {@code application/x-www-form-urlencoded}, or
* {@code multipart/form-data}; anything else is rejected with 415.
*/
@Provider
public class JsonContentTypeFilter implements ContainerRequestFilter {

private static final Set<String> ALLOWED_TYPES = Set.of(
"application/json",
"application/x-www-form-urlencoded",
"multipart/form-data");
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

@Copilot Renamed to AllowedContentTypeFilter in 1e3b2d1.

tbonelee
tbonelee previously approved these changes May 9, 2026
Copy link
Copy Markdown
Contributor

@tbonelee tbonelee left a comment

Choose a reason for hiding this comment

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

LGTM, approving. A couple of small suggestions you might consider:

  1. Could we rename JsonContentTypeFilter? It also accepts form-urlencoded and multipart, so something like MediaTypeAllowlistFilter might describe it better. Probably easier to do before merge.
  2. The new allowed.origins default and Content-Type rejection will break some external clients on upgrade. It'd be great to have these called out in the next release notes.

jongyoul added 3 commits May 14, 2026 18:45
Apply stricter defaults to the request-handling layer:

- CorsFilter blocks state-changing methods (POST/PUT/DELETE/PATCH) and
  cross-origin preflight requests when the Origin header is not in the
  configured allow-list. Access-Control-Allow-Credentials is only sent
  when the Origin is allowed.
- The default value of zeppelin.server.allowed.origins changes from "*"
  to empty so cross-origin browser access must be explicitly enabled.
  Operators relying on the previous default need to set this back to "*"
  or to specific origin(s). Same-origin/same-host and non-browser
  clients are unaffected.
- A new request filter restricts REST request bodies on state-changing
  methods to application/json, application/x-www-form-urlencoded, or
  multipart/form-data; other media types are rejected with 415.
- The default shiro.ini.template now sets cookie.sameSite = LAX.
- ZeppelinClient.addParagraph and updateParagraph now send an explicit
  Content-Type: application/json header so they pass the new filter.
- CorsUtils.isValidOrigin normalizes the Origin header to lowercase
  before the allow-list membership check, mirroring how the configured
  origins are stored, so case differences in the Origin header do not
  produce false rejections.
- A small HttpMethods utility holds the shared STATE_CHANGING method
  set used by both the servlet filter and the Jersey filter.
ZeppelinClient state-changing calls and the AbstractTestRestApi PUT
helper now declare application/json so the new content-type filter
accepts them. Aligns the rest of the surface with addParagraph and
updateParagraph, which already set the header.
@jongyoul jongyoul force-pushed the minor-cors-hardening branch from 5f65bce to 396b3fd Compare May 14, 2026 09:46
jongyoul and others added 2 commits May 15, 2026 13:29
…limit test

%252525252e is 5 layers of URL-encoding for ".", which decodes
successfully within MAX_DECODE_LAYERS=5. The test must use 6 layers
(%25252525252e) to actually trigger "Exceeded maximum decode attempts".

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Use Locale.ROOT when lowercasing allowed-origins config so the
  CorsUtils membership check stays locale-independent
  (Turkish locale could previously cause false rejections).
- Catch UnknownHostException in CorsFilter so a misconfigured local
  hostname surfaces as a deterministic 403 instead of a 500.
- Rename JsonContentTypeFilter to AllowedContentTypeFilter to match
  the broader allow-list it actually enforces (JSON, form-urlencoded,
  multipart).

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@jongyoul
Copy link
Copy Markdown
Member Author

@tbonelee Thanks for the review! Addressed both suggestions:

  1. JsonContentTypeFilter is now AllowedContentTypeFilter (1e3b2d1) — agreed the previous name was misleading once form-urlencoded and multipart were in the allow-list.
  2. Good point on the breaking change. The default zeppelin.server.allowed.origins now being empty and the Content-Type allow-list both deserve a callout — I will make sure these are mentioned in the next release notes for 0.13.0.

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.

3 participants