Copilot commented on code in PR #11712:
URL: https://github.com/apache/gravitino/pull/11712#discussion_r3426955177
##########
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
can expose sensitive values in plaintext. See
+[Catalog Properties
Configuration](./gravitino-server-config.md#catalog-properties-configuration)
for details.
Review Comment:
The warning sentence is split across two lines with a hard newline after
“See”, which can read awkwardly in rendered markdown. Consider making this a
single sentence on one line, or (preferably) using a `:::warning` admonition
(as done in `gravitino-server-config.md`) so this security guidance is visually
prominent and consistently formatted.
##########
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
can expose sensitive values in plaintext. See
+[Catalog Properties
Configuration](./gravitino-server-config.md#catalog-properties-configuration)
for details.
Review Comment:
This warning text is duplicated verbatim in multiple places in the same
document. To reduce the risk of future drift, consider standardizing via a
shared admonition block (same wording/format in both locations) or referencing
a single canonical warning section if your documentation tooling supports
includes/partials.
##########
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, tokens, or access
keys can expose
+sensitive values in plaintext, because these properties are not fully managed
by Gravitino and may
+be returned in REST API responses. If an underlying system requires
credentials to be passed this
+way, restrict access to the related REST APIs.
+:::
Review Comment:
The warning explains the risk well, but the phrasing “returned in REST API
responses” is slightly ambiguous about the exposure mechanism. Consider
tightening it to explicitly state that these values may be returned *in
plaintext* via REST API responses, aligning with the earlier “plaintext”
wording and making the security outcome unambiguous.
--
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]
