Copilot commented on code in PR #11712:
URL: https://github.com/apache/gravitino/pull/11712#discussion_r3426614784
##########
docs/lakehouse-iceberg-catalog.md:
##########
@@ -219,6 +221,8 @@ For other storages that are not managed by Gravitino
directly, you can manage th
| `io-impl` | The IO implementation for `FileIO` in Iceberg. Use the
fully qualified class name to override the default implementation. |
`org.apache.iceberg.io.ResolvingFileIO` | No | 0.6.0-incubating |
To pass custom properties such as `security-token` to your custom `FileIO`,
you can directly configure it by `gravitino.bypass.security-token`.
`security-token` will be included in the properties when the initialize method
of `FileIO` is invoked.
+Passing credentials or security tokens through `gravitino.bypass.` properties
has a credential leakage risk. See
+[Catalog Properties
Configuration](./gravitino-server-config.md#catalog-properties-configuration)
for details.
Review Comment:
Same wording issue as above: "has a credential leakage risk" reads
awkwardly. Rephrase to clearly state that sensitive values can be exposed.
##########
docs/gravitino-server-config.md:
##########
@@ -392,6 +392,14 @@ There are three types of catalog properties:
configurations for the catalog to work properly.
2. **Properties with the `gravitino.bypass.` prefix**: These properties are
not managed by
Gravitino and pass directly to the underlying system for advanced usage.
+
+:::warning
+Using `gravitino.bypass.` properties to pass credentials has a risk of
credential leakage, because
+these properties are not fully managed by Gravitino and may be returned in
REST API responses as
+plain text. If an underlying system requires credentials to be passed this
way, restrict access to
+the related REST APIs.
Review Comment:
The warning text could be clearer/stronger and align with other docs by
explicitly mentioning that tokens/access keys are also sensitive, and by using
"plaintext" / "can leak sensitive values" phrasing instead of "has a risk of
credential leakage".
##########
docs/lakehouse-iceberg-catalog.md:
##########
@@ -161,6 +161,8 @@ Gravitino Iceberg REST service supports using static
access-key-id and secret-ac
| `oss-endpoint` | The endpoint of Aliyun OSS service.
|
(none) | No | 0.7.0-incubating |
For other Iceberg OSS properties not managed by Gravitino like
`client.security-token`, you could config it directly by
`gravitino.bypass.client.security-token`.
+Passing credentials or security tokens through `gravitino.bypass.` properties
has a credential leakage risk. See
+[Catalog Properties
Configuration](./gravitino-server-config.md#catalog-properties-configuration)
for details.
Review Comment:
The phrase "has a credential leakage risk" is awkward/unclear. Consider
rephrasing to explicitly state the impact (that sensitive values can leak)
while keeping the link intact.
--
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]
