This is an automated email from the ASF dual-hosted git repository.
diqiu50 pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/gravitino.git
The following commit(s) were added to refs/heads/main by this push:
new 17e7b77283 [#14231] fix(secret): Omit reserved+hidden properties from
getSecrets (#14232)
17e7b77283 is described below
commit 17e7b77283a6e4c84e8ee919450bc78b5dc71e59
Author: MaSai <[email protected]>
AuthorDate: Mon Oct 26 19:06:50 2026 +0800
[#14231] fix(secret): Omit reserved+hidden properties from getSecrets
(#14232)
### What changes were proposed in this pull request?
- Exclude reserved+hidden properties (e.g. `gravitino.identifier`) from
`SecretPropertyUtils.shouldRecoverSensitiveNamedSecret` / `buildSecrets`
- Align docs with `HiddenPropertyMaskUtils` omit semantics
- Add unit coverage
### Why are the changes needed?
`getSecrets` treated every declared hidden property as recoverable, so
the internal `gravitino.identifier` leaked into secret responses.
Fix: #14231
### Does this PR introduce _any_ user-facing change?
Yes — `getSecrets` no longer returns reserved+hidden internal keys such
as `gravitino.identifier`. Real hidden secrets (e.g. `jdbc-password`)
are unchanged.
### How was this patch tested?
`./gradlew :core:test --tests
org.apache.gravitino.secret.TestSecretPropertyUtils -PskipITs`
Co-authored-by: Cursor <[email protected]>
---
.../apache/gravitino/secret/SupportsSecrets.java | 7 +++--
.../connector/HiddenPropertyMaskUtils.java | 8 +++---
.../gravitino/secret/SecretPropertyUtils.java | 20 +++++++++-----
.../gravitino/secret/TestSecretPropertyUtils.java | 32 ++++++++++++++++++++++
4 files changed, 53 insertions(+), 14 deletions(-)
diff --git a/api/src/main/java/org/apache/gravitino/secret/SupportsSecrets.java
b/api/src/main/java/org/apache/gravitino/secret/SupportsSecrets.java
index db87435dfa..3f835733e8 100644
--- a/api/src/main/java/org/apache/gravitino/secret/SupportsSecrets.java
+++ b/api/src/main/java/org/apache/gravitino/secret/SupportsSecrets.java
@@ -28,9 +28,10 @@ import java.util.Map;
* <ul>
* <li>Every secret-URN property value, resolved via the secret manager
(including keys that may
* also be delivered via {@link
org.apache.gravitino.credential.SupportsCredentials}).
- * <li>Stored plaintext for every property declared {@code hidden} in entity
metadata (no
- * sensitive-name keyword gate), so shortening the server sensitive-key
keyword list cannot
- * leave a masked property without a recovery path.
+ * <li>Stored plaintext for every property declared {@code hidden} but not
{@code reserved} in
+ * entity metadata (no sensitive-name keyword gate), so shortening the
server sensitive-key
+ * keyword list cannot leave a masked property without a recovery path.
Reserved+hidden
+ * internal keys such as {@code gravitino.identifier} are never returned.
* <li>Stored plaintext for undeclared keys whose names look sensitive
(contain {@code secret},
* {@code password}, {@code token}, {@code credential}, {@code access},
or {@code account},
* case-insensitive), so mistyped / undeclared credential properties
remain usable after API
diff --git
a/core/src/main/java/org/apache/gravitino/connector/HiddenPropertyMaskUtils.java
b/core/src/main/java/org/apache/gravitino/connector/HiddenPropertyMaskUtils.java
index 8301e73639..e4f407fc56 100644
---
a/core/src/main/java/org/apache/gravitino/connector/HiddenPropertyMaskUtils.java
+++
b/core/src/main/java/org/apache/gravitino/connector/HiddenPropertyMaskUtils.java
@@ -56,10 +56,10 @@ import org.slf4j.LoggerFactory;
* in-use}, {@code numFiles}).
* </ul>
*
- * <p>Recovery via {@code getSecrets} covers secret-URN values, declared
{@code hidden} keys (except
- * credential-vending keys), and sensitive-named undeclared keys. Credential
property keys are
- * recovered via {@code getCredentials()} instead. See {@code
- * org.apache.gravitino.secret.SupportsSecrets}.
+ * <p>Recovery via {@code getSecrets} covers secret-URN values, declared
{@code hidden} keys that
+ * are not also {@code reserved} (reserved+hidden keys such as {@code
gravitino.identifier} stay
+ * omitted), and sensitive-named undeclared keys. Credential property keys are
recovered via {@code
+ * getCredentials()} instead. See {@code
org.apache.gravitino.secret.SupportsSecrets}.
*
* <p><b>Write (create / alter)</b>: reject any value equal to {@link
#MASKED_VALUE} so the
* placeholder is never persisted. Reserved / immutable rejection remains in
{@code
diff --git
a/core/src/main/java/org/apache/gravitino/secret/SecretPropertyUtils.java
b/core/src/main/java/org/apache/gravitino/secret/SecretPropertyUtils.java
index 5b4f6341e0..710eb28b27 100644
--- a/core/src/main/java/org/apache/gravitino/secret/SecretPropertyUtils.java
+++ b/core/src/main/java/org/apache/gravitino/secret/SecretPropertyUtils.java
@@ -89,9 +89,12 @@ public final class SecretPropertyUtils {
* <ul>
* <li>{@code metadata == null}: do <strong>not</strong> recover
(URN-only). Used when the
* catalog does not expose properties metadata for the entity type.
- * <li>declared {@code hidden}: always recover, without the
sensitive-keyword gate — so
- * shortening {@link Configs#SENSITIVE_KEY_KEYWORDS} cannot leave a
masked property without
- * a recovery path.
+ * <li>declared {@code hidden} and not {@code reserved}: always recover,
without the
+ * sensitive-keyword gate — so shortening {@link
Configs#SENSITIVE_KEY_KEYWORDS} cannot
+ * leave a masked property without a recovery path.
+ * <li>declared {@code reserved} and {@code hidden} (for example {@code
gravitino.identifier}):
+ * never recover — these are internal system properties omitted from
API responses, not user
+ * secrets.
* <li>undeclared: recover only when the key name matches {@link
#isSensitivePropertyKey}
* (historical fuzzy recovery).
* <li>declared non-hidden: never recover here (for example {@code
credential-providers} stays
@@ -112,7 +115,9 @@ public final class SecretPropertyUtils {
return false;
}
if (metadata.containsProperty(key)) {
- return metadata.isHiddenProperty(key);
+ // Align with HiddenPropertyMaskUtils: reserved+hidden keys are internal
(e.g.
+ // gravitino.identifier) and must not appear in getSecrets.
+ return metadata.isHiddenProperty(key) &&
!metadata.isReservedProperty(key);
}
return isSensitivePropertyKey(key);
}
@@ -154,9 +159,10 @@ public final class SecretPropertyUtils {
* <li>Include every entry where {@link #isSecretProperty} is true,
resolving the secret URN via
* {@link SecretManager#readSecret}.
* <li>Include every inline entry when {@link
#shouldRecoverSensitiveNamedSecret} is true:
- * declared {@code hidden} (no keyword gate), or undeclared with a
sensitive-looking name.
- * Declared non-hidden keys are excluded even when the name looks
sensitive. When {@code
- * metadata} is {@code null}, inline plaintext is not recovered
(URN-only).
+ * declared {@code hidden} and not {@code reserved} (no keyword gate),
or undeclared with a
+ * sensitive-looking name. Reserved+hidden keys (for example {@code
gravitino.identifier})
+ * and declared non-hidden keys are excluded. When {@code metadata} is
{@code null}, inline
+ * plaintext is not recovered (URN-only).
* </ol>
*
* <p>Normal non-sensitive, non-hidden properties are not included. Clients
merge this map over
diff --git
a/core/src/test/java/org/apache/gravitino/secret/TestSecretPropertyUtils.java
b/core/src/test/java/org/apache/gravitino/secret/TestSecretPropertyUtils.java
index 97c67d5bdf..85b7a53f20 100644
---
a/core/src/test/java/org/apache/gravitino/secret/TestSecretPropertyUtils.java
+++
b/core/src/test/java/org/apache/gravitino/secret/TestSecretPropertyUtils.java
@@ -18,6 +18,8 @@
*/
package org.apache.gravitino.secret;
+import static org.apache.gravitino.StringIdentifier.ID_KEY;
+
import com.google.common.collect.ImmutableMap;
import java.util.HashMap;
import java.util.List;
@@ -254,6 +256,36 @@ public class TestSecretPropertyUtils {
}
}
+ @Test
+ void testBuildSecretsOmitsReservedHiddenProperties() {
+ try (SecretManager sm = memorySecretManager()) {
+ PropertiesMetadata metadata =
+ new PropertiesMetadata() {
+ @Override
+ public Map<String, PropertyEntry<?>> propertyEntries() {
+ return ImmutableMap.of(
+ ID_KEY,
+ PropertyEntry.stringReservedPropertyEntry(
+ ID_KEY, "internal entity identifier", true),
+ "jdbc-password",
+ PropertyEntry.stringOptionalPropertyEntry(
+ "jdbc-password", "password", false, null, true));
+ }
+ };
+ Map<String, String> entityProps =
+ Map.of(ID_KEY, "gravitino.v1.uid123", "jdbc-password", "s3cr3t");
+ Map<String, String> secrets = SecretPropertyUtils.buildSecrets(sm,
entityProps, metadata);
+ Assertions.assertEquals("s3cr3t", secrets.get("jdbc-password"));
+ Assertions.assertFalse(
+ secrets.containsKey(ID_KEY),
+ "reserved+hidden gravitino.identifier must not appear in
getSecrets");
+ Assertions.assertFalse(
+ SecretPropertyUtils.shouldRecoverSensitiveNamedSecret(ID_KEY,
metadata));
+ Assertions.assertTrue(
+
SecretPropertyUtils.shouldRecoverSensitiveNamedSecret("jdbc-password",
metadata));
+ }
+ }
+
@Test
void testMergeProperties() {
Map<String, String> merged =
