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]