Copilot commented on code in PR #12626:
URL: https://github.com/apache/gravitino/pull/12626#discussion_r4121706695
##########
server/src/main/java/org/apache/gravitino/server/web/rest/SemanticModelOperations.java:
##########
@@ -63,6 +72,44 @@ public SemanticModelOperations(SemanticModelDispatcher
dispatcher) {
this.dispatcher = dispatcher;
}
+ /**
+ * Lists Semantic Models in a schema.
+ *
+ * @param metalake The metalake name.
+ * @param catalog The catalog name.
+ * @param schema The schema name.
+ * @return A response containing Semantic Model identifiers.
+ */
+ @GET
+ @Produces("application/vnd.gravitino.v1+json")
+ @Timed(name = "list-semantic-model." + MetricNames.HTTP_PROCESS_DURATION,
absolute = true)
+ @ResponseMetered(name = "list-semantic-model", absolute = true)
+ public Response listSemanticModels(
Review Comment:
`SemanticModelOperations` is not among the REST classes registered by
`GravitinoInterceptionService`, and this method has no authorization
annotations of its own. Consequently the new list endpoint is not intercepted
at all, so an authenticated caller can enumerate Semantic Models in schemas
they cannot access. Register this resource and add the appropriate schema/list
authorization and metadata bindings.
This issue also appears on line 83 of the same file.
##########
server/src/main/java/org/apache/gravitino/server/web/rest/SemanticModelOperations.java:
##########
@@ -63,6 +72,44 @@ public SemanticModelOperations(SemanticModelDispatcher
dispatcher) {
this.dispatcher = dispatcher;
}
+ /**
+ * Lists Semantic Models in a schema.
+ *
+ * @param metalake The metalake name.
+ * @param catalog The catalog name.
+ * @param schema The schema name.
+ * @return A response containing Semantic Model identifiers.
+ */
+ @GET
+ @Produces("application/vnd.gravitino.v1+json")
+ @Timed(name = "list-semantic-model." + MetricNames.HTTP_PROCESS_DURATION,
absolute = true)
+ @ResponseMetered(name = "list-semantic-model", absolute = true)
+ public Response listSemanticModels(
+ @PathParam("metalake") String metalake,
+ @PathParam("catalog") String catalog,
+ @PathParam("schema") String schema) {
+ LOG.info(
+ "Received list Semantic Models request for schema: {}.{}.{}",
metalake, catalog, schema);
+ try {
+ return Utils.doAs(
+ httpRequest,
+ () -> {
+ Namespace namespace = NamespaceUtil.ofSemanticModel(metalake,
catalog, schema);
+ NameIdentifier[] identifiers =
dispatcher.listSemanticModels(namespace);
+ identifiers = identifiers == null ? new NameIdentifier[0] :
identifiers;
Review Comment:
This returns the complete dispatcher result without any authorization
filtering. The Semantic Model authorization contract requires list/load to
expose only models visible through SELECT/MODIFY/ownership, but this endpoint
has no authorization annotation or `MetadataAuthzHelper` filter, so callers can
enumerate unauthorized model names. Add the semantic-model list authorization
expression and filter the identifiers before constructing `EntityListResponse`.
##########
server/src/main/java/org/apache/gravitino/server/web/rest/SemanticModelOperations.java:
##########
@@ -157,4 +204,103 @@ public Response loadSemanticModel(
OperationType.LOAD, semanticModel, schema, e);
}
}
+
+ /**
+ * Alters a Semantic Model atomically.
+ *
+ * @param metalake The metalake name.
+ * @param catalog The catalog name.
+ * @param schema The schema name.
+ * @param semanticModel The current Semantic Model name.
+ * @param request The updates to apply.
+ * @return A response containing the altered Semantic Model.
+ */
+ @PUT
+ @Path("{semanticModel}")
+ @Produces("application/vnd.gravitino.v1+json")
+ @Timed(name = "alter-semantic-model." + MetricNames.HTTP_PROCESS_DURATION,
absolute = true)
+ @ResponseMetered(name = "alter-semantic-model", absolute = true)
+ public Response alterSemanticModel(
Review Comment:
This new alter route has no authorization check, unlike the other
schema-scoped metadata alter endpoints. A caller who can reach the REST
resource can therefore rename, change properties, or replace a definition
without the required MODIFY_SEMANTIC_MODEL privilege or ownership. Add the
appropriate semantic-model authorization expression and authorization metadata
to this method before dispatching the changes.
This issue also appears on line 269 of the same file.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]