roryqi commented on code in PR #13778:
URL: https://github.com/apache/gravitino/pull/13778#discussion_r4333870279
##########
iceberg/iceberg-rest-server/src/main/java/org/apache/gravitino/iceberg/service/rest/IcebergTableOperations.java:
##########
@@ -343,14 +346,20 @@ public Response loadTable(
LoadTableResponse loadTableResponse =
tableOperationDispatcher.loadTable(context, tableIdentifier);
+ if
(IcebergRESTUtils.SnapshotMode.REFS.getValue().equals(snapshots)) {
+ loadTableResponse = filterSnapshotsByRefs(loadTableResponse);
+ }
+ IcebergReadRestrictionLoader.Result restrictionResult =
+ readRestrictionLoader.load(context, tableIdentifier,
loadTableResponse);
+ loadTableResponse = restrictionResult.response();
Review Comment:
Thanks for raising this. The security model of this PR is reader-enforced
restrictions: Gravitino returns the effective restrictions in loadTable, and a
reader that supports read-restrictions must apply them to every read made using
that response. The upstream REST spec explicitly includes subsequent
planTableScan and fetchScanTasks calls in that scope:
https://github.com/apache/iceberg/blob/main/open-api/rest-catalog-open-api.yaml#L1001-L1008
The current spec defines read-restrictions on LoadTableResult, while the
plan/tasks result schemas contain scan tasks and have no read-restrictions
field. It does not require those endpoints to repeat the restrictions.
Therefore, a compliant reader that loaded the table does see the row filter and
masks before using server-side scan planning.
The scope matters: Gravitino currently adds restrictions only to loadTable.
Neither the spec nor this PR makes direct plan/tasks calls, clients that ignore
restrictions, or direct storage reads a server-enforced security boundary.
Preventing those cases would require a separate design for scan authorization
and storage access. The PR description already states that deployments using
this experimental service trust clients to enforce returned restrictions. I
agree that create/update responses are a separate gap to consider because they
also return LoadTableResponse, although those operations require write
privileges.
--
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]
