roryqi commented on code in PR #14508:
URL: https://github.com/apache/gravitino/pull/14508#discussion_r4464905057


##########
core/src/main/java/org/apache/gravitino/catalog/SchemaOperationDispatcher.java:
##########
@@ -684,13 +725,15 @@ private void importSchema(NameIdentifier identifier) {
       }
     }
 
+    // Persist source properties, not the masked client view, so a recovered 
registration retains
+    // secret URNs. The EntityCombinedSchema returned to the caller still 
applies masking.
+    Map<String, String> sourceProperties = schema.schema().properties();
     SchemaEntity schemaEntity =
         SchemaEntity.builder()
             .withId(uid)
             .withName(identifier.name())
             .withNamespace(identifier.namespace())
-            .withProperties(
-                schema.properties() == null ? Collections.emptyMap() : 
schema.properties())
+            .withProperties(sourceProperties == null ? Collections.emptyMap() 
: sourceProperties)

Review Comment:
   **[P1] Validate write-through secret URN ownership before importing raw 
properties.**
   
   If an external schema B copies A's secret URN but omits 
`gravitino.identifier`, this import generates a new ID for B and persists A's 
URN without hitting the existing schema-identity conflict check. 
`getSecrets(B)` then authorizes access against B and resolves A's credential. 
Dropping B also passes the copied URN to `deleteSecretsFromProperties`, which 
checks only that it has three identifier segments and deletes A's still-used 
secret. Previously the masked property map did not persist this URN.
   
   Could we validate the URN's entity type, ID and property key against the 
imported schema, using `isWriteThroughForEntity`, before accepting a 
write-through URN? A regression covering a copied URN with the schema marker 
removed should verify that B cannot expose or delete A's secret. Matching URNs 
from a partial-create recovery should remain supported.
   
   Found by static review; tests were not run.



-- 
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]

Reply via email to