From 18cd6d4728b31d05b13b904dc2ce7b5cce0cae3e Mon Sep 17 00:00:00 2001 From: "xiaohongbo.xhb" Date: Sat, 18 Jul 2026 20:28:11 -0700 Subject: [PATCH 01/13] [api] Redact sensitive config values in logs and REST exceptions Add SensitiveConfigUtils with isSensitive / redactValue / redactMap / redactText. Sensitive values (password, secret, token, credential, access-key, authorization, ...) are masked, keeping the last 4 chars of long values to aid troubleshooting. Apply it on the REST transport: HttpClient no longer echoes an unparsed error response body verbatim into the exception, and RESTUtil no longer interpolates the request body value on encode failure (keeps the body type and Jackson's original reason instead). Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_01MWKFWMMt1jgU76Fy9x1PgC --- .../org/apache/paimon/rest/HttpClient.java | 7 +- .../java/org/apache/paimon/rest/RESTUtil.java | 7 +- .../paimon/utils/SensitiveConfigUtils.java | 131 ++++++++++++++++++ .../utils/SensitiveConfigUtilsTest.java | 124 +++++++++++++++++ 4 files changed, 267 insertions(+), 2 deletions(-) create mode 100644 paimon-api/src/main/java/org/apache/paimon/utils/SensitiveConfigUtils.java create mode 100644 paimon-api/src/test/java/org/apache/paimon/utils/SensitiveConfigUtilsTest.java diff --git a/paimon-api/src/main/java/org/apache/paimon/rest/HttpClient.java b/paimon-api/src/main/java/org/apache/paimon/rest/HttpClient.java index 2477ff9db69b..67268223cd9f 100644 --- a/paimon-api/src/main/java/org/apache/paimon/rest/HttpClient.java +++ b/paimon-api/src/main/java/org/apache/paimon/rest/HttpClient.java @@ -24,6 +24,7 @@ import org.apache.paimon.rest.exceptions.RESTException; import org.apache.paimon.rest.interceptor.LoggingInterceptor; import org.apache.paimon.rest.responses.ErrorResponse; +import org.apache.paimon.utils.SensitiveConfigUtils; import org.apache.paimon.utils.StringUtils; import org.apache.paimon.shade.jackson2.com.fasterxml.jackson.core.JsonProcessingException; @@ -236,7 +237,11 @@ private static ErrorResponse buildErrorResponse( (error != null && error.getResourceName() != null) ? error.getResourceName() : ""; - String message = responseBodyStr != null ? responseBodyStr : "response body is null"; + // Redact: unparsed body is echoed into the exception. + String message = + responseBodyStr != null + ? SensitiveConfigUtils.redactText(responseBodyStr) + : "response body is null"; int code = (error != null && error.getCode() != null) ? error.getCode() : errorCode; error = new ErrorResponse(resourceType, resourceName, message, code); } diff --git a/paimon-api/src/main/java/org/apache/paimon/rest/RESTUtil.java b/paimon-api/src/main/java/org/apache/paimon/rest/RESTUtil.java index 8a5e760793b7..17db07484630 100644 --- a/paimon-api/src/main/java/org/apache/paimon/rest/RESTUtil.java +++ b/paimon-api/src/main/java/org/apache/paimon/rest/RESTUtil.java @@ -167,7 +167,12 @@ public static String encodedBody(Object body) { try { return RESTApi.toJson(body); } catch (JsonProcessingException e) { - throw new RESTException(e, "Failed to encode request body: %s", body); + // Body may hold credentials; keep type + Jackson reason, drop the value. + throw new RESTException( + e, + "Failed to encode request body of type %s: %s", + body.getClass().getName(), + e.getOriginalMessage()); } } return null; diff --git a/paimon-api/src/main/java/org/apache/paimon/utils/SensitiveConfigUtils.java b/paimon-api/src/main/java/org/apache/paimon/utils/SensitiveConfigUtils.java new file mode 100644 index 000000000000..2a7f6a5ad424 --- /dev/null +++ b/paimon-api/src/main/java/org/apache/paimon/utils/SensitiveConfigUtils.java @@ -0,0 +1,131 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.paimon.utils; + +import java.util.LinkedHashMap; +import java.util.Locale; +import java.util.Map; +import java.util.regex.Matcher; +import java.util.regex.Pattern; + +/** + * Redacts sensitive configuration values (passwords, secrets, tokens, access keys) before they are + * written to logs or exception messages. Only protects Paimon-produced output; third-party logs + * (Hive, Hadoop, cloud SDKs) must be handled by the runtime logging config. + */ +public class SensitiveConfigUtils { + + /** Placeholder for a fully-masked (short) sensitive value. */ + public static final String REDACTED = "******"; + + /** Values at least this long keep their last {@link #TAIL_LEN} chars to aid troubleshooting. */ + private static final int MIN_LEN_FOR_TAIL = 12; + + private static final int TAIL_LEN = 4; + + /** + * Substrings that mark a configuration key as sensitive. Keys are normalized (lower-cased with + * separators removed) before matching, so {@code fs.oss.accessKeySecret}, {@code + * fs.s3a.access.key} and {@code fs.s3a.secret.key} are all detected. + */ + private static final String[] SENSITIVE_KEY_PATTERNS = { + "password", + "secret", + "token", + "credential", + "accesskey", + "authorization", + "privatekey", + "apikey" + }; + + /** Matches {@code key:value} / {@code key=value} pairs whose key looks sensitive. */ + private static final Pattern SENSITIVE_TEXT = + Pattern.compile( + "(?i)([\"']?[\\w.-]*" + + "(?:secret|token|password|credential|access[._-]?key|authorization" + + "|api[._-]?key|private[._-]?key)" + + "[\\w.-]*[\"']?\\s*[:=]\\s*)([\"']?)([^\"',&}\\s]+)"); + + private SensitiveConfigUtils() {} + + /** Returns whether the given configuration key is considered sensitive. */ + public static boolean isSensitive(String key) { + if (key == null || key.isEmpty()) { + return false; + } + String normalized = key.toLowerCase(Locale.ROOT).replaceAll("[^a-z0-9]", ""); + for (String pattern : SENSITIVE_KEY_PATTERNS) { + if (normalized.contains(pattern)) { + return true; + } + } + return false; + } + + /** Returns the value masked if its key is sensitive, otherwise the value unchanged. */ + public static String redactValue(String key, String value) { + return isSensitive(key) ? maskValue(value) : value; + } + + /** + * Returns a copy of the map with every sensitive value masked. Insertion order is preserved. + * Safe to pass {@code null}. + */ + public static Map redactMap(Map options) { + if (options == null) { + return null; + } + Map redacted = new LinkedHashMap<>(options.size()); + for (Map.Entry entry : options.entrySet()) { + String key = entry.getKey(); + redacted.put(key, isSensitive(key) ? maskValue(entry.getValue()) : entry.getValue()); + } + return redacted; + } + + /** + * Best-effort masking of secret values in free-form text (JSON/form HTTP bodies): masks the + * value after any sensitive-looking key. Prefer {@link #redactMap} when keys are known. + */ + public static String redactText(String text) { + if (text == null || text.isEmpty()) { + return text; + } + Matcher matcher = SENSITIVE_TEXT.matcher(text); + StringBuffer sb = new StringBuffer(); + while (matcher.find()) { + String replacement = matcher.group(1) + matcher.group(2) + maskValue(matcher.group(3)); + matcher.appendReplacement(sb, Matcher.quoteReplacement(replacement)); + } + matcher.appendTail(sb); + return sb.toString(); + } + + /** Masks a value, keeping the last {@link #TAIL_LEN} chars when it is long enough. */ + private static String maskValue(String value) { + if (value == null) { + return null; + } + if (value.length() >= MIN_LEN_FOR_TAIL) { + return "****" + value.substring(value.length() - TAIL_LEN); + } + return REDACTED; + } +} diff --git a/paimon-api/src/test/java/org/apache/paimon/utils/SensitiveConfigUtilsTest.java b/paimon-api/src/test/java/org/apache/paimon/utils/SensitiveConfigUtilsTest.java new file mode 100644 index 000000000000..962ab3d05e85 --- /dev/null +++ b/paimon-api/src/test/java/org/apache/paimon/utils/SensitiveConfigUtilsTest.java @@ -0,0 +1,124 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.paimon.utils; + +import org.junit.jupiter.api.Test; + +import java.util.LinkedHashMap; +import java.util.Map; + +import static org.apache.paimon.utils.SensitiveConfigUtils.REDACTED; +import static org.assertj.core.api.Assertions.assertThat; + +/** Tests for {@link SensitiveConfigUtils}. */ +class SensitiveConfigUtilsTest { + + @Test + void testIsSensitiveDetectsCredentialKeys() { + // Access keys / secrets in various vendor spellings. + assertThat(SensitiveConfigUtils.isSensitive("fs.oss.accessKeyId")).isTrue(); + assertThat(SensitiveConfigUtils.isSensitive("fs.oss.accessKeySecret")).isTrue(); + assertThat(SensitiveConfigUtils.isSensitive("fs.s3a.access.key")).isTrue(); + assertThat(SensitiveConfigUtils.isSensitive("fs.s3a.secret.key")).isTrue(); + assertThat(SensitiveConfigUtils.isSensitive("fs.s3a.session.token")).isTrue(); + assertThat(SensitiveConfigUtils.isSensitive("security.token")).isTrue(); + assertThat(SensitiveConfigUtils.isSensitive("my.password")).isTrue(); + assertThat(SensitiveConfigUtils.isSensitive("some.credential")).isTrue(); + assertThat(SensitiveConfigUtils.isSensitive("Authorization")).isTrue(); + assertThat(SensitiveConfigUtils.isSensitive("client.private-key")).isTrue(); + assertThat(SensitiveConfigUtils.isSensitive("dlf.api-key")).isTrue(); + } + + @Test + void testIsSensitiveIgnoresNonCredentialKeys() { + assertThat(SensitiveConfigUtils.isSensitive("bucket")).isFalse(); + assertThat(SensitiveConfigUtils.isSensitive("warehouse")).isFalse(); + assertThat(SensitiveConfigUtils.isSensitive("fs.oss.endpoint")).isFalse(); + assertThat(SensitiveConfigUtils.isSensitive("metastore")).isFalse(); + assertThat(SensitiveConfigUtils.isSensitive("")).isFalse(); + assertThat(SensitiveConfigUtils.isSensitive(null)).isFalse(); + } + + @Test + void testRedactValue() { + // Long value keeps only its last 4 chars. + assertThat(SensitiveConfigUtils.redactValue("fs.oss.accessKeySecret", "0123456789abcdef")) + .isEqualTo("****cdef"); + // Short value is fully masked. + assertThat(SensitiveConfigUtils.redactValue("password", "short")).isEqualTo(REDACTED); + // Non-sensitive key is untouched. + assertThat(SensitiveConfigUtils.redactValue("bucket", "my-bucket")).isEqualTo("my-bucket"); + } + + @Test + void testRedactMapReplacesOnlySensitiveValues() { + Map options = new LinkedHashMap<>(); + options.put("warehouse", "oss://bucket/warehouse"); + options.put("fs.oss.accessKeyId", "LTAI-secret-id-01"); + options.put("fs.oss.accessKeySecret", "real-secret-value"); + options.put("fs.oss.endpoint", "oss-cn-hangzhou.aliyuncs.com"); + + Map redacted = SensitiveConfigUtils.redactMap(options); + + assertThat(redacted).containsEntry("warehouse", "oss://bucket/warehouse"); + // Long secrets keep their last 4 chars; endpoint is untouched. + assertThat(redacted).containsEntry("fs.oss.accessKeyId", "****d-01"); + assertThat(redacted).containsEntry("fs.oss.accessKeySecret", "****alue"); + assertThat(redacted).containsEntry("fs.oss.endpoint", "oss-cn-hangzhou.aliyuncs.com"); + // Original map must not be mutated. + assertThat(options).containsEntry("fs.oss.accessKeySecret", "real-secret-value"); + // The rendered string must not contain any raw secret. + assertThat(redacted.toString()) + .doesNotContain("real-secret-value") + .doesNotContain("LTAI-secret-id-01"); + } + + @Test + void testRedactMapNullSafe() { + assertThat(SensitiveConfigUtils.redactMap(null)).isNull(); + } + + @Test + void testRedactTextMasksSecretsInJsonAndForm() { + String json = + "{\"accessKeyId\":\"LTAI-id\",\"accessKeySecret\":\"the-real-secret\"," + + "\"securityToken\":\"tok-123\",\"endpoint\":\"oss-cn-hangzhou\"}"; + String redactedJson = SensitiveConfigUtils.redactText(json); + assertThat(redactedJson) + .doesNotContain("the-real-secret") + .doesNotContain("tok-123") + .contains(REDACTED) + // Non-sensitive fields are kept. + .contains("oss-cn-hangzhou"); + + String form = "password=hunter2&user=alice&fs.oss.access-key=AK-9"; + String redactedForm = SensitiveConfigUtils.redactText(form); + assertThat(redactedForm) + .doesNotContain("hunter2") + .doesNotContain("AK-9") + .contains("user=alice"); + } + + @Test + void testRedactTextNullAndEmptySafe() { + assertThat(SensitiveConfigUtils.redactText(null)).isNull(); + assertThat(SensitiveConfigUtils.redactText("")).isEmpty(); + assertThat(SensitiveConfigUtils.redactText("no secrets here")).isEqualTo("no secrets here"); + } +} From ba384d05961c6d571dfb658f5435aefe52188ffe Mon Sep 17 00:00:00 2001 From: "xiaohongbo.xhb" Date: Sat, 18 Jul 2026 20:28:11 -0700 Subject: [PATCH 02/13] [common][flink] Redact sensitive config values in Hadoop and Flink logs HadoopUtils no longer logs the value of forwarded hadoop.* entries at DEBUG. FlinkTableSource and ConfigRefresher route table/option maps through SensitiveConfigUtils.redactMap before logging, so credentials (fs.oss.*, fs.s3.*, ...) are masked. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_01MWKFWMMt1jgU76Fy9x1PgC --- .../org/apache/paimon/utils/HadoopUtils.java | 6 +- .../utils/HadoopUtilsSecretLoggingTest.java | 83 +++++++++++++++++++ .../paimon/flink/sink/ConfigRefresher.java | 5 +- .../paimon/flink/source/FlinkTableSource.java | 6 +- 4 files changed, 94 insertions(+), 6 deletions(-) create mode 100644 paimon-common/src/test/java/org/apache/paimon/utils/HadoopUtilsSecretLoggingTest.java diff --git a/paimon-common/src/main/java/org/apache/paimon/utils/HadoopUtils.java b/paimon-common/src/main/java/org/apache/paimon/utils/HadoopUtils.java index a37c64dbb847..6da7d457ab30 100644 --- a/paimon-common/src/main/java/org/apache/paimon/utils/HadoopUtils.java +++ b/paimon-common/src/main/java/org/apache/paimon/utils/HadoopUtils.java @@ -140,11 +140,11 @@ public static Configuration getHadoopConfiguration(Options options) { String newKey = key.substring(prefix.length()); String value = options.getString(key, null); result.set(newKey, value); + // Do not log value: may contain credentials. LOG.debug( - "Adding Paimon config entry for {} as {}={} to Hadoop config", + "Adding Paimon config entry for {} as {} to Hadoop config", key, - newKey, - value); + newKey); foundHadoopConfiguration = true; } } diff --git a/paimon-common/src/test/java/org/apache/paimon/utils/HadoopUtilsSecretLoggingTest.java b/paimon-common/src/test/java/org/apache/paimon/utils/HadoopUtilsSecretLoggingTest.java new file mode 100644 index 000000000000..fe323feaf6ba --- /dev/null +++ b/paimon-common/src/test/java/org/apache/paimon/utils/HadoopUtilsSecretLoggingTest.java @@ -0,0 +1,83 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.paimon.utils; + +import org.apache.paimon.options.Options; + +import org.apache.hadoop.conf.Configuration; +import org.apache.logging.log4j.Level; +import org.apache.logging.log4j.LogManager; +import org.apache.logging.log4j.core.Logger; +import org.apache.logging.log4j.core.appender.OutputStreamAppender; +import org.apache.logging.log4j.core.layout.PatternLayout; +import org.junit.jupiter.api.Test; + +import java.io.ByteArrayOutputStream; +import java.nio.charset.StandardCharsets; + +import static org.assertj.core.api.Assertions.assertThat; + +/** + * Verifies that {@link HadoopUtils#getHadoopConfiguration} forwards {@code hadoop.*} credentials to + * the Hadoop configuration without leaking their values into the (DEBUG) logs. + */ +class HadoopUtilsSecretLoggingTest { + + private static final String SECRET_MARKER = "UNIQUE-SECRET-MARKER-9f8e7d6c"; + + @Test + void testCredentialValueIsNotLogged() { + Options options = new Options(); + // hadoop.* keys are forwarded (prefix stripped) into the Hadoop configuration. + options.set("hadoop.fs.oss.accessKeySecret", SECRET_MARKER); + options.set("hadoop.fs.oss.endpoint", "oss-cn-hangzhou.aliyuncs.com"); + + // Warm up: first Hadoop Configuration build reconfigures log4j2 and drops our appender. + HadoopUtils.getHadoopConfiguration(new Options()); + + ByteArrayOutputStream output = new ByteArrayOutputStream(); + OutputStreamAppender appender = + OutputStreamAppender.newBuilder() + .setName("hadoop-utils-secret-test") + .setTarget(output) + .setLayout(PatternLayout.newBuilder().withPattern("%level %msg%n").build()) + .build(); + Logger logger = (Logger) LogManager.getLogger(HadoopUtils.class); + Level previousLevel = logger.getLevel(); + + appender.start(); + logger.addAppender(appender); + logger.setLevel(Level.DEBUG); + try { + Configuration conf = HadoopUtils.getHadoopConfiguration(options); + + // The credential must still be forwarded so the configuration keeps working. + assertThat(conf.get("fs.oss.accessKeySecret")).isEqualTo(SECRET_MARKER); + + String logs = new String(output.toByteArray(), StandardCharsets.UTF_8); + // The key name may appear, but the secret value must never be logged. + assertThat(logs).contains("fs.oss.accessKeySecret"); + assertThat(logs).doesNotContain(SECRET_MARKER); + } finally { + logger.setLevel(previousLevel); + logger.removeAppender(appender); + appender.stop(); + } + } +} diff --git a/paimon-flink/paimon-flink-common/src/main/java/org/apache/paimon/flink/sink/ConfigRefresher.java b/paimon-flink/paimon-flink-common/src/main/java/org/apache/paimon/flink/sink/ConfigRefresher.java index 1d4b8deb083e..917b851ca687 100644 --- a/paimon-flink/paimon-flink-common/src/main/java/org/apache/paimon/flink/sink/ConfigRefresher.java +++ b/paimon-flink/paimon-flink-common/src/main/java/org/apache/paimon/flink/sink/ConfigRefresher.java @@ -23,6 +23,7 @@ import org.apache.paimon.options.Options; import org.apache.paimon.schema.TableSchema; import org.apache.paimon.table.FileStoreTable; +import org.apache.paimon.utils.SensitiveConfigUtils; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -101,8 +102,8 @@ public void tryRefresh() { refresher.refresh(table); LOG.info( "write has been refreshed due to configs changed. old options:{}, new options:{}.", - currentOptions, - newOptions); + SensitiveConfigUtils.redactMap(currentOptions), + SensitiveConfigUtils.redactMap(newOptions)); } } catch (Exception e) { throw new RuntimeException("update write failed.", e); diff --git a/paimon-flink/paimon-flink-common/src/main/java/org/apache/paimon/flink/source/FlinkTableSource.java b/paimon-flink/paimon-flink-common/src/main/java/org/apache/paimon/flink/source/FlinkTableSource.java index 66cb49798aa0..534783fb02b3 100644 --- a/paimon-flink/paimon-flink-common/src/main/java/org/apache/paimon/flink/source/FlinkTableSource.java +++ b/paimon-flink/paimon-flink-common/src/main/java/org/apache/paimon/flink/source/FlinkTableSource.java @@ -37,6 +37,7 @@ import org.apache.paimon.table.Table; import org.apache.paimon.table.source.Split; import org.apache.paimon.utils.RowDataToObjectArrayConverter; +import org.apache.paimon.utils.SensitiveConfigUtils; import org.apache.flink.configuration.Configuration; import org.apache.flink.streaming.api.environment.StreamExecutionEnvironment; @@ -180,7 +181,10 @@ private PartitionPredicate getPartitionPredicateWithOptions() { // In older versions of Flink, however, lookup sources will first be treated as normal // sources. So this method will also be visited by lookup tables, and the options might // cause IllegalArgumentException. In this case we ignore the filters. - LOG.info("Failed to get filter with table options {} ", table.options(), e); + LOG.info( + "Failed to get filter with table options {} ", + SensitiveConfigUtils.redactMap(table.options()), + e); return null; } } From bb617876416d59df0df40e99d053ef67c0fc5def Mon Sep 17 00:00:00 2001 From: "xiaohongbo.xhb" Date: Sat, 18 Jul 2026 20:41:49 -0700 Subject: [PATCH 03/13] [common] Mask instead of drop the value in Hadoop config debug log HadoopUtils now logs the forwarded hadoop.* value through SensitiveConfigUtils.redactValue, so non-sensitive entries keep their value while credentials are masked, instead of dropping every value. Use mock values in the redaction tests. --- .../utils/SensitiveConfigUtilsTest.java | 36 +++++++++---------- .../org/apache/paimon/utils/HadoopUtils.java | 7 ++-- .../utils/HadoopUtilsSecretLoggingTest.java | 2 +- 3 files changed, 23 insertions(+), 22 deletions(-) diff --git a/paimon-api/src/test/java/org/apache/paimon/utils/SensitiveConfigUtilsTest.java b/paimon-api/src/test/java/org/apache/paimon/utils/SensitiveConfigUtilsTest.java index 962ab3d05e85..994615f48847 100644 --- a/paimon-api/src/test/java/org/apache/paimon/utils/SensitiveConfigUtilsTest.java +++ b/paimon-api/src/test/java/org/apache/paimon/utils/SensitiveConfigUtilsTest.java @@ -69,24 +69,24 @@ void testRedactValue() { @Test void testRedactMapReplacesOnlySensitiveValues() { Map options = new LinkedHashMap<>(); - options.put("warehouse", "oss://bucket/warehouse"); - options.put("fs.oss.accessKeyId", "LTAI-secret-id-01"); - options.put("fs.oss.accessKeySecret", "real-secret-value"); - options.put("fs.oss.endpoint", "oss-cn-hangzhou.aliyuncs.com"); + options.put("warehouse", "mock://warehouse"); + options.put("fs.oss.accessKeyId", "mock-access-id-0001"); + options.put("fs.oss.accessKeySecret", "mock-secret-value"); + options.put("fs.oss.endpoint", "mock-endpoint.example.com"); Map redacted = SensitiveConfigUtils.redactMap(options); - assertThat(redacted).containsEntry("warehouse", "oss://bucket/warehouse"); + assertThat(redacted).containsEntry("warehouse", "mock://warehouse"); // Long secrets keep their last 4 chars; endpoint is untouched. - assertThat(redacted).containsEntry("fs.oss.accessKeyId", "****d-01"); + assertThat(redacted).containsEntry("fs.oss.accessKeyId", "****0001"); assertThat(redacted).containsEntry("fs.oss.accessKeySecret", "****alue"); - assertThat(redacted).containsEntry("fs.oss.endpoint", "oss-cn-hangzhou.aliyuncs.com"); + assertThat(redacted).containsEntry("fs.oss.endpoint", "mock-endpoint.example.com"); // Original map must not be mutated. - assertThat(options).containsEntry("fs.oss.accessKeySecret", "real-secret-value"); + assertThat(options).containsEntry("fs.oss.accessKeySecret", "mock-secret-value"); // The rendered string must not contain any raw secret. assertThat(redacted.toString()) - .doesNotContain("real-secret-value") - .doesNotContain("LTAI-secret-id-01"); + .doesNotContain("mock-secret-value") + .doesNotContain("mock-access-id-0001"); } @Test @@ -97,21 +97,21 @@ void testRedactMapNullSafe() { @Test void testRedactTextMasksSecretsInJsonAndForm() { String json = - "{\"accessKeyId\":\"LTAI-id\",\"accessKeySecret\":\"the-real-secret\"," - + "\"securityToken\":\"tok-123\",\"endpoint\":\"oss-cn-hangzhou\"}"; + "{\"accessKeyId\":\"mock-id\",\"accessKeySecret\":\"mock-secret-abcd\"," + + "\"securityToken\":\"mock-tok\",\"endpoint\":\"mock-endpoint\"}"; String redactedJson = SensitiveConfigUtils.redactText(json); assertThat(redactedJson) - .doesNotContain("the-real-secret") - .doesNotContain("tok-123") + .doesNotContain("mock-secret-abcd") + .doesNotContain("mock-tok") .contains(REDACTED) // Non-sensitive fields are kept. - .contains("oss-cn-hangzhou"); + .contains("mock-endpoint"); - String form = "password=hunter2&user=alice&fs.oss.access-key=AK-9"; + String form = "password=mock-pass&user=alice&fs.oss.access-key=mock-ak"; String redactedForm = SensitiveConfigUtils.redactText(form); assertThat(redactedForm) - .doesNotContain("hunter2") - .doesNotContain("AK-9") + .doesNotContain("mock-pass") + .doesNotContain("mock-ak") .contains("user=alice"); } diff --git a/paimon-common/src/main/java/org/apache/paimon/utils/HadoopUtils.java b/paimon-common/src/main/java/org/apache/paimon/utils/HadoopUtils.java index 6da7d457ab30..eb21b4eb8754 100644 --- a/paimon-common/src/main/java/org/apache/paimon/utils/HadoopUtils.java +++ b/paimon-common/src/main/java/org/apache/paimon/utils/HadoopUtils.java @@ -140,11 +140,12 @@ public static Configuration getHadoopConfiguration(Options options) { String newKey = key.substring(prefix.length()); String value = options.getString(key, null); result.set(newKey, value); - // Do not log value: may contain credentials. + // Redact value: sensitive keys are masked, others printed as-is. LOG.debug( - "Adding Paimon config entry for {} as {} to Hadoop config", + "Adding Paimon config entry for {} as {}={} to Hadoop config", key, - newKey); + newKey, + SensitiveConfigUtils.redactValue(newKey, value)); foundHadoopConfiguration = true; } } diff --git a/paimon-common/src/test/java/org/apache/paimon/utils/HadoopUtilsSecretLoggingTest.java b/paimon-common/src/test/java/org/apache/paimon/utils/HadoopUtilsSecretLoggingTest.java index fe323feaf6ba..7f092b8a865e 100644 --- a/paimon-common/src/test/java/org/apache/paimon/utils/HadoopUtilsSecretLoggingTest.java +++ b/paimon-common/src/test/java/org/apache/paimon/utils/HadoopUtilsSecretLoggingTest.java @@ -46,7 +46,7 @@ void testCredentialValueIsNotLogged() { Options options = new Options(); // hadoop.* keys are forwarded (prefix stripped) into the Hadoop configuration. options.set("hadoop.fs.oss.accessKeySecret", SECRET_MARKER); - options.set("hadoop.fs.oss.endpoint", "oss-cn-hangzhou.aliyuncs.com"); + options.set("hadoop.fs.oss.endpoint", "mock-endpoint.example.com"); // Warm up: first Hadoop Configuration build reconfigures log4j2 and drops our appender. HadoopUtils.getHadoopConfiguration(new Options()); From f58e89cc4c43d756a6a4c029ffff78078b7ca676 Mon Sep 17 00:00:00 2001 From: "xiaohongbo.xhb" Date: Sat, 18 Jul 2026 22:13:48 -0700 Subject: [PATCH 04/13] [api] Harden secret redaction in REST error handling and logs Address several credential-leak paths: - redactText masked only the first whitespace-delimited word, leaking the token in "Authorization: Bearer "; the value now runs to a quote/comma/brace/ampersand/newline so multi-word values are masked whole. - Passwords, tokens and Authorization values are now fully masked (******) instead of keeping a 4-char tail. - Sensitive-key detection now also covers Azure/S3 credentials (fs.azure.account.key.*, fs.azure.sas.*, fs.s3a.encryption.key). - HttpClient.buildErrorResponse redacts the parsed ErrorResponse.message (previously only the unparsed fallback body was handled) and no longer echoes an arbitrary unparseable body into the exception. - A 2xx response that fails to deserialize now throws a type-only RESTException without the raw body or Jackson's source snippet, which could contain credentials (e.g. token responses). --- .../org/apache/paimon/rest/HttpClient.java | 46 +++++++------ .../paimon/utils/SensitiveConfigUtils.java | 49 ++++++++++---- .../utils/SensitiveConfigUtilsTest.java | 39 ++++++++++- .../apache/paimon/rest/HttpClientTest.java | 66 ++++++++++++++++--- 4 files changed, 157 insertions(+), 43 deletions(-) diff --git a/paimon-api/src/main/java/org/apache/paimon/rest/HttpClient.java b/paimon-api/src/main/java/org/apache/paimon/rest/HttpClient.java index 67268223cd9f..73c7890b3868 100644 --- a/paimon-api/src/main/java/org/apache/paimon/rest/HttpClient.java +++ b/paimon-api/src/main/java/org/apache/paimon/rest/HttpClient.java @@ -139,12 +139,21 @@ private T exec(HttpUriRequestBase request, Class res } catch (JsonProcessingException e) { // ignore exception } - error = buildErrorResponse(error, responseBodyStr, response.getCode()); + error = buildErrorResponse(error, response.getCode()); errorHandler.accept(error, extractRequestId(response)); } if (responseType != null && responseBodyStr != null) { - return RESTApi.fromJson(responseBodyStr, responseType); + try { + return RESTApi.fromJson(responseBodyStr, responseType); + } catch (JsonProcessingException e) { + // Do not surface the body or Jackson's source snippet: a + // successful response (e.g. a token response) may contain + // credentials. Report only the target type. + throw new RESTException( + "Failed to parse REST response into %s", + responseType.getName()); + } } else if (responseType == null) { return null; } else { @@ -226,25 +235,20 @@ private static Header[] getHeaders( .toArray(Header[]::new); } - private static ErrorResponse buildErrorResponse( - ErrorResponse error, String responseBodyStr, int errorCode) { - if (error == null || error.getMessage() == null || error.getMessage().isEmpty()) { - String resourceType = - (error != null && error.getResourceType() != null) - ? error.getResourceType() - : ""; - String resourceName = - (error != null && error.getResourceName() != null) - ? error.getResourceName() - : ""; - // Redact: unparsed body is echoed into the exception. - String message = - responseBodyStr != null - ? SensitiveConfigUtils.redactText(responseBodyStr) - : "response body is null"; - int code = (error != null && error.getCode() != null) ? error.getCode() : errorCode; - error = new ErrorResponse(resourceType, resourceName, message, code); + private static ErrorResponse buildErrorResponse(ErrorResponse error, int errorCode) { + String resourceType = + (error != null && error.getResourceType() != null) ? error.getResourceType() : ""; + String resourceName = + (error != null && error.getResourceName() != null) ? error.getResourceName() : ""; + int code = (error != null && error.getCode() != null) ? error.getCode() : errorCode; + String message; + if (error != null && error.getMessage() != null && !error.getMessage().isEmpty()) { + // A parsed message may still embed secrets (e.g. "password=..."); redact it. + message = SensitiveConfigUtils.redactText(error.getMessage()); + } else { + // The raw body is arbitrary and cannot be reliably sanitized; do not echo it. + message = "Unparseable error response body (HTTP " + errorCode + ")."; } - return error; + return new ErrorResponse(resourceType, resourceName, message, code); } } diff --git a/paimon-api/src/main/java/org/apache/paimon/utils/SensitiveConfigUtils.java b/paimon-api/src/main/java/org/apache/paimon/utils/SensitiveConfigUtils.java index 2a7f6a5ad424..4d7af1d186b5 100644 --- a/paimon-api/src/main/java/org/apache/paimon/utils/SensitiveConfigUtils.java +++ b/paimon-api/src/main/java/org/apache/paimon/utils/SensitiveConfigUtils.java @@ -42,7 +42,8 @@ public class SensitiveConfigUtils { /** * Substrings that mark a configuration key as sensitive. Keys are normalized (lower-cased with * separators removed) before matching, so {@code fs.oss.accessKeySecret}, {@code - * fs.s3a.access.key} and {@code fs.s3a.secret.key} are all detected. + * fs.s3a.access.key}, {@code fs.azure.account.key.*}, {@code fs.azure.sas.*} and {@code + * fs.s3a.encryption.key} are all detected. */ private static final String[] SENSITIVE_KEY_PATTERNS = { "password", @@ -50,28 +51,45 @@ public class SensitiveConfigUtils { "token", "credential", "accesskey", + "accountkey", + "encryptionkey", "authorization", "privatekey", - "apikey" + "apikey", + "sas" }; - /** Matches {@code key:value} / {@code key=value} pairs whose key looks sensitive. */ + /** + * Keys whose value is fully masked instead of keeping a trailing hint. Passwords and bearer + * tokens leak too much from even a few characters, so they are never partially revealed. + */ + private static final String[] FULL_MASK_KEY_PATTERNS = {"password", "token", "authorization"}; + + /** + * Matches {@code key:value} / {@code key=value} pairs whose key looks sensitive. The value runs + * up to a quote, comma, brace, ampersand or line break, so multi-token values such as {@code + * Authorization: Bearer } are masked whole rather than only their first word. + */ private static final Pattern SENSITIVE_TEXT = Pattern.compile( "(?i)([\"']?[\\w.-]*" - + "(?:secret|token|password|credential|access[._-]?key|authorization" - + "|api[._-]?key|private[._-]?key)" - + "[\\w.-]*[\"']?\\s*[:=]\\s*)([\"']?)([^\"',&}\\s]+)"); + + "(?:secret|token|password|credential|access[._-]?key|account[._-]?key" + + "|encryption[._-]?key|authorization|api[._-]?key|private[._-]?key|sas)" + + "[\\w.-]*[\"']?\\s*[:=]\\s*)([\"']?)([^\"',&}\\r\\n]+)"); private SensitiveConfigUtils() {} /** Returns whether the given configuration key is considered sensitive. */ public static boolean isSensitive(String key) { + return matchesAny(key, SENSITIVE_KEY_PATTERNS); + } + + private static boolean matchesAny(String key, String[] patterns) { if (key == null || key.isEmpty()) { return false; } String normalized = key.toLowerCase(Locale.ROOT).replaceAll("[^a-z0-9]", ""); - for (String pattern : SENSITIVE_KEY_PATTERNS) { + for (String pattern : patterns) { if (normalized.contains(pattern)) { return true; } @@ -81,7 +99,7 @@ public static boolean isSensitive(String key) { /** Returns the value masked if its key is sensitive, otherwise the value unchanged. */ public static String redactValue(String key, String value) { - return isSensitive(key) ? maskValue(value) : value; + return isSensitive(key) ? maskValue(value, matchesAny(key, FULL_MASK_KEY_PATTERNS)) : value; } /** @@ -95,7 +113,7 @@ public static Map redactMap(Map options) { Map redacted = new LinkedHashMap<>(options.size()); for (Map.Entry entry : options.entrySet()) { String key = entry.getKey(); - redacted.put(key, isSensitive(key) ? maskValue(entry.getValue()) : entry.getValue()); + redacted.put(key, redactValue(key, entry.getValue())); } return redacted; } @@ -111,19 +129,24 @@ public static String redactText(String text) { Matcher matcher = SENSITIVE_TEXT.matcher(text); StringBuffer sb = new StringBuffer(); while (matcher.find()) { - String replacement = matcher.group(1) + matcher.group(2) + maskValue(matcher.group(3)); + boolean fullMask = matchesAny(matcher.group(1), FULL_MASK_KEY_PATTERNS); + String replacement = + matcher.group(1) + matcher.group(2) + maskValue(matcher.group(3), fullMask); matcher.appendReplacement(sb, Matcher.quoteReplacement(replacement)); } matcher.appendTail(sb); return sb.toString(); } - /** Masks a value, keeping the last {@link #TAIL_LEN} chars when it is long enough. */ - private static String maskValue(String value) { + /** + * Masks a value. Password/token keys are fully masked; other secrets keep their last {@link + * #TAIL_LEN} chars when long enough, to aid troubleshooting. + */ + private static String maskValue(String value, boolean fullMask) { if (value == null) { return null; } - if (value.length() >= MIN_LEN_FOR_TAIL) { + if (!fullMask && value.length() >= MIN_LEN_FOR_TAIL) { return "****" + value.substring(value.length() - TAIL_LEN); } return REDACTED; diff --git a/paimon-api/src/test/java/org/apache/paimon/utils/SensitiveConfigUtilsTest.java b/paimon-api/src/test/java/org/apache/paimon/utils/SensitiveConfigUtilsTest.java index 994615f48847..430a7e00b6f2 100644 --- a/paimon-api/src/test/java/org/apache/paimon/utils/SensitiveConfigUtilsTest.java +++ b/paimon-api/src/test/java/org/apache/paimon/utils/SensitiveConfigUtilsTest.java @@ -43,6 +43,12 @@ void testIsSensitiveDetectsCredentialKeys() { assertThat(SensitiveConfigUtils.isSensitive("Authorization")).isTrue(); assertThat(SensitiveConfigUtils.isSensitive("client.private-key")).isTrue(); assertThat(SensitiveConfigUtils.isSensitive("dlf.api-key")).isTrue(); + // Azure / S3 cloud credentials. + assertThat(SensitiveConfigUtils.isSensitive("fs.azure.account.key.acct.blob.core")) + .isTrue(); + assertThat(SensitiveConfigUtils.isSensitive("fs.azure.sas.container.acct.blob.core")) + .isTrue(); + assertThat(SensitiveConfigUtils.isSensitive("fs.s3a.encryption.key")).isTrue(); } @Test @@ -57,7 +63,7 @@ void testIsSensitiveIgnoresNonCredentialKeys() { @Test void testRedactValue() { - // Long value keeps only its last 4 chars. + // Secret/access-key values keep only their last 4 chars. assertThat(SensitiveConfigUtils.redactValue("fs.oss.accessKeySecret", "0123456789abcdef")) .isEqualTo("****cdef"); // Short value is fully masked. @@ -66,12 +72,27 @@ void testRedactValue() { assertThat(SensitiveConfigUtils.redactValue("bucket", "my-bucket")).isEqualTo("my-bucket"); } + @Test + void testPasswordAndTokenAreFullyMasked() { + // Passwords and tokens leak too much from a trailing hint -> always fully masked. + assertThat(SensitiveConfigUtils.redactValue("my.password", "0123456789abcdef")) + .isEqualTo(REDACTED); + assertThat(SensitiveConfigUtils.redactValue("security.token", "0123456789abcdef")) + .isEqualTo(REDACTED); + assertThat(SensitiveConfigUtils.redactValue("Authorization", "Bearer 0123456789abcdef")) + .isEqualTo(REDACTED); + // Contrast: an access-key secret keeps its tail. + assertThat(SensitiveConfigUtils.redactValue("fs.s3a.secret.key", "0123456789abcdef")) + .isEqualTo("****cdef"); + } + @Test void testRedactMapReplacesOnlySensitiveValues() { Map options = new LinkedHashMap<>(); options.put("warehouse", "mock://warehouse"); options.put("fs.oss.accessKeyId", "mock-access-id-0001"); options.put("fs.oss.accessKeySecret", "mock-secret-value"); + options.put("fs.azure.account.key.acct", "mock-azure-key-value"); options.put("fs.oss.endpoint", "mock-endpoint.example.com"); Map redacted = SensitiveConfigUtils.redactMap(options); @@ -80,6 +101,7 @@ void testRedactMapReplacesOnlySensitiveValues() { // Long secrets keep their last 4 chars; endpoint is untouched. assertThat(redacted).containsEntry("fs.oss.accessKeyId", "****0001"); assertThat(redacted).containsEntry("fs.oss.accessKeySecret", "****alue"); + assertThat(redacted).containsEntry("fs.azure.account.key.acct", "****alue"); assertThat(redacted).containsEntry("fs.oss.endpoint", "mock-endpoint.example.com"); // Original map must not be mutated. assertThat(options).containsEntry("fs.oss.accessKeySecret", "mock-secret-value"); @@ -115,6 +137,21 @@ void testRedactTextMasksSecretsInJsonAndForm() { .contains("user=alice"); } + @Test + void testRedactTextMasksWholeValueWithSpaces() { + // The token after the "Bearer " scheme (with a space) must be masked, not just "Bearer". + String header = "Authorization: Bearer mock-jwt-token-abcdef"; + assertThat(SensitiveConfigUtils.redactText(header)) + .doesNotContain("mock-jwt-token-abcdef") + .startsWith("Authorization: "); + + // A quoted multi-word secret value is masked up to the closing quote only. + String json = "{\"token\":\"Bearer mock-jwt-abcd\",\"user\":\"alice\"}"; + assertThat(SensitiveConfigUtils.redactText(json)) + .doesNotContain("mock-jwt-abcd") + .contains("\"user\":\"alice\""); + } + @Test void testRedactTextNullAndEmptySafe() { assertThat(SensitiveConfigUtils.redactText(null)).isNull(); diff --git a/paimon-core/src/test/java/org/apache/paimon/rest/HttpClientTest.java b/paimon-core/src/test/java/org/apache/paimon/rest/HttpClientTest.java index a7424190372d..dadfd409379d 100644 --- a/paimon-core/src/test/java/org/apache/paimon/rest/HttpClientTest.java +++ b/paimon-core/src/test/java/org/apache/paimon/rest/HttpClientTest.java @@ -23,6 +23,7 @@ import org.apache.paimon.rest.auth.RESTAuthFunction; import org.apache.paimon.rest.auth.RESTAuthParameter; import org.apache.paimon.rest.exceptions.BadRequestException; +import org.apache.paimon.rest.exceptions.RESTException; import org.apache.paimon.rest.responses.ErrorResponse; import org.apache.paimon.shade.guava30.com.google.common.collect.ImmutableMap; @@ -43,6 +44,7 @@ import static org.junit.jupiter.api.Assertions.assertDoesNotThrow; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertThrows; /** Test for {@link HttpClient}. */ @@ -130,6 +132,51 @@ public void testGetFail() { () -> httpClient.get(MOCK_PATH, MockRESTData.class, restAuthFunction)); } + @Test + public void testErrorResponseMessageIsRedacted() throws Exception { + // A parsed ErrorResponse whose message carries a secret must not leak it. + String secret = "SUPERSECRETVALUE9999"; + String body = + server.createResponseBody( + new ErrorResponse( + ErrorResponse.RESOURCE_TYPE_DATABASE, + "test", + "token=" + secret, + 400)); + server.enqueueResponse(body, 400); + BadRequestException e = + assertThrows( + BadRequestException.class, + () -> httpClient.get(MOCK_PATH, MockRESTData.class, restAuthFunction)); + assertFalse(e.getMessage().contains(secret)); + } + + @Test + public void testUnparseableErrorBodyIsNotEchoed() { + // A non-JSON error body cannot be reliably sanitized, so it must not be echoed at all. + String secret = "SUPERSECRETVALUE9999"; + server.enqueueResponse("token=" + secret + " <>", 400); + BadRequestException e = + assertThrows( + BadRequestException.class, + () -> httpClient.get(MOCK_PATH, MockRESTData.class, restAuthFunction)); + assertFalse(e.getMessage().contains(secret)); + } + + @Test + public void testSuccessResponseParseFailureDoesNotLeakBody() { + // A 2xx body that fails to deserialize (e.g. a token response) must not surface the + // raw body or Jackson's source snippet. + String secret = "SUPERSECRETVALUE9999"; + String body = "{\"data\": \"token=" + secret + "\" INVALID_JSON}"; + server.enqueueResponse(body, 200); + RESTException e = + assertThrows( + RESTException.class, + () -> httpClient.get(MOCK_PATH, MockRESTData.class, restAuthFunction)); + assertFalse(e.getMessage().contains(secret)); + } + @Test public void testPostSuccess() { server.enqueueResponse(mockResponseDataStr, 200); @@ -228,8 +275,8 @@ private Map getParameters(String path) { @Test public void testGetWithUnparsableJsonErrorResponse() { - // Test case for JSON response with mismatched field names that cannot be parsed as - // ErrorResponse + // A JSON body that cannot be mapped to ErrorResponse must NOT be echoed (it may carry + // secrets); the exception carries only a generic message with the HTTP status. String jsonWithUppercaseFields = "{\"Message\":\"Your request is denied as lack of ssl protect.\"," + "\"Code\":\"InvalidProtocol.NeedSsl\"}"; @@ -239,16 +286,18 @@ public void testGetWithUnparsableJsonErrorResponse() { httpClient.get(MOCK_PATH, MockRESTData.class, restAuthFunction); Assertions.fail("Expected exception to be thrown"); } catch (Exception e) { - Assertions.assertTrue( + Assertions.assertFalse( e.getMessage().contains("Your request is denied as lack of ssl protect") || e.getMessage().contains(jsonWithUppercaseFields), - "Error message should contain the original response body"); + "Raw response body must not be echoed"); + Assertions.assertTrue( + e.getMessage().contains("403"), "Message should carry the HTTP status"); } } @Test public void testPostWithNonJsonErrorResponse() { - // Test case for non-JSON response (plain text) that cannot be parsed + // A non-JSON (plain text) error body must NOT be echoed; only a generic message remains. String plainTextResponse = "Internal Server Error: Database connection failed"; server.enqueueResponse(plainTextResponse, 500); @@ -256,11 +305,12 @@ public void testPostWithNonJsonErrorResponse() { httpClient.post(MOCK_PATH, mockResponseData, MockRESTData.class, restAuthFunction); Assertions.fail("Expected exception to be thrown"); } catch (Exception e) { - // Verify that the error message contains the original plain text response - Assertions.assertTrue( + Assertions.assertFalse( e.getMessage().contains(plainTextResponse) || e.getMessage().contains("Database connection failed"), - "Error message should contain the original non-JSON response"); + "Raw response body must not be echoed"); + Assertions.assertTrue( + e.getMessage().contains("500"), "Message should carry the HTTP status"); } } From 02823a88f9a3c3f74ddedced21a7635cb8e06166 Mon Sep 17 00:00:00 2001 From: "xiaohongbo.xhb" Date: Sun, 19 Jul 2026 00:42:50 -0700 Subject: [PATCH 05/13] [api] Fully mask secrets (AWS-aligned), harden redactText, cover more log/URI leaks Follow-up hardening after review: - Fully mask every true secret (secret, credential, private-key, encryption-key, api-key, account-key, sas, in addition to password / token / authorization); only identifier-like accessKeyId keeps a trailing hint, mirroring AWS/Azure where the access-key id is loggable but the secret is never shown. accessKeySecret normalizes to contain "secret" so it is fully masked; accessKeyId is not. - Replace redactText's regex with a linear all-or-nothing scan: it had a ReDoS (O(n^2) on ~20KB input) and leaked past commas / quotes / spaces and Azure sig. Any sensitive marker now redacts the whole text. - Add SensitiveConfigUtils.sanitizeUri and strip the query/user-info in LoggingInterceptor and HttpClientUtils.exists so signed-URL credentials (AWS/GCS signature, Azure SAS sig, presigned tokens) do not reach logs or exceptions. - RESTUtil encode-failure keeps only the body type: neither the cause nor Jackson's message is safe when a getter throws with a secret. - Redact dynamic table options in AbstractFlinkTableFactory and the hadoop.* config values logged by OSS/OBS/COSN/Jindo/GS FileIO. --- .../apache/paimon/rest/HttpClientUtils.java | 6 +- .../java/org/apache/paimon/rest/RESTUtil.java | 8 +- .../rest/interceptor/LoggingInterceptor.java | 4 +- .../paimon/utils/SensitiveConfigUtils.java | 112 ++++++++++++++---- .../utils/SensitiveConfigUtilsTest.java | 94 +++++++-------- .../org/apache/paimon/cosn/COSNFileIO.java | 3 +- .../java/org/apache/paimon/gs/GSFileIO.java | 3 +- .../org/apache/paimon/jindo/JindoFileIO.java | 3 +- .../java/org/apache/paimon/obs/OBSFileIO.java | 3 +- .../java/org/apache/paimon/oss/OSSFileIO.java | 3 +- .../flink/AbstractFlinkTableFactory.java | 3 +- 11 files changed, 154 insertions(+), 88 deletions(-) diff --git a/paimon-api/src/main/java/org/apache/paimon/rest/HttpClientUtils.java b/paimon-api/src/main/java/org/apache/paimon/rest/HttpClientUtils.java index f54ef7044db0..d35b81e9d2ba 100644 --- a/paimon-api/src/main/java/org/apache/paimon/rest/HttpClientUtils.java +++ b/paimon-api/src/main/java/org/apache/paimon/rest/HttpClientUtils.java @@ -20,6 +20,7 @@ import org.apache.paimon.rest.interceptor.LoggingInterceptor; import org.apache.paimon.rest.interceptor.TimingInterceptor; +import org.apache.paimon.utils.SensitiveConfigUtils; import org.apache.hc.client5.http.classic.methods.HttpGet; import org.apache.hc.client5.http.classic.methods.HttpHead; @@ -120,7 +121,10 @@ public static boolean exists(String uri) throws IOException { return false; } throw new IOException( - "Unexpected HTTP status code: " + rangeStatusCode + " for uri: " + uri); + "Unexpected HTTP status code: " + + rangeStatusCode + + " for uri: " + + SensitiveConfigUtils.sanitizeUri(uri)); } public static boolean isNotFoundError(Throwable throwable) { diff --git a/paimon-api/src/main/java/org/apache/paimon/rest/RESTUtil.java b/paimon-api/src/main/java/org/apache/paimon/rest/RESTUtil.java index 17db07484630..d7ee78f511df 100644 --- a/paimon-api/src/main/java/org/apache/paimon/rest/RESTUtil.java +++ b/paimon-api/src/main/java/org/apache/paimon/rest/RESTUtil.java @@ -167,12 +167,10 @@ public static String encodedBody(Object body) { try { return RESTApi.toJson(body); } catch (JsonProcessingException e) { - // Body may hold credentials; keep type + Jackson reason, drop the value. + // Keep only the body type: a throwing getter can surface the secret in both + // the cause chain and Jackson's message, so neither is safe to include. throw new RESTException( - e, - "Failed to encode request body of type %s: %s", - body.getClass().getName(), - e.getOriginalMessage()); + "Failed to encode request body of type %s", body.getClass().getName()); } } return null; diff --git a/paimon-api/src/main/java/org/apache/paimon/rest/interceptor/LoggingInterceptor.java b/paimon-api/src/main/java/org/apache/paimon/rest/interceptor/LoggingInterceptor.java index 8422944dca5c..51c31bb6f69c 100644 --- a/paimon-api/src/main/java/org/apache/paimon/rest/interceptor/LoggingInterceptor.java +++ b/paimon-api/src/main/java/org/apache/paimon/rest/interceptor/LoggingInterceptor.java @@ -18,6 +18,8 @@ package org.apache.paimon.rest.interceptor; +import org.apache.paimon.utils.SensitiveConfigUtils; + import org.apache.hc.core5.http.EntityDetails; import org.apache.hc.core5.http.HttpRequest; import org.apache.hc.core5.http.HttpResponse; @@ -55,7 +57,7 @@ public void process( "[rest] requestId:{} method:{} url:{} duration:{}ms", requestId, request.getMethod(), - request.getUri(), + SensitiveConfigUtils.sanitizeUri(request.getUri().toString()), durationMs); } catch (URISyntaxException e) { LOG.warn("Failed to log rest request: {}", e.getMessage()); diff --git a/paimon-api/src/main/java/org/apache/paimon/utils/SensitiveConfigUtils.java b/paimon-api/src/main/java/org/apache/paimon/utils/SensitiveConfigUtils.java index 4d7af1d186b5..e4dc33681363 100644 --- a/paimon-api/src/main/java/org/apache/paimon/utils/SensitiveConfigUtils.java +++ b/paimon-api/src/main/java/org/apache/paimon/utils/SensitiveConfigUtils.java @@ -18,11 +18,11 @@ package org.apache.paimon.utils; +import java.net.URI; +import java.net.URISyntaxException; import java.util.LinkedHashMap; import java.util.Locale; import java.util.Map; -import java.util.regex.Matcher; -import java.util.regex.Pattern; /** * Redacts sensitive configuration values (passwords, secrets, tokens, access keys) before they are @@ -60,22 +60,49 @@ public class SensitiveConfigUtils { }; /** - * Keys whose value is fully masked instead of keeping a trailing hint. Passwords and bearer - * tokens leak too much from even a few characters, so they are never partially revealed. + * Keys whose value is fully masked instead of keeping a trailing hint. Every true secret is + * here; only identifier-like keys ({@code accessKeyId}) keep a tail. This mirrors AWS/Azure, + * where the access-key id is loggable but the secret / token / SAS is never shown. Note {@code + * accessKeySecret} normalizes to contain {@code secret}, so it is fully masked while {@code + * accessKeyId} (only {@code accesskey}) keeps its tail. */ - private static final String[] FULL_MASK_KEY_PATTERNS = {"password", "token", "authorization"}; + private static final String[] FULL_MASK_KEY_PATTERNS = { + "password", + "token", + "authorization", + "secret", + "credential", + "privatekey", + "encryptionkey", + "apikey", + "accountkey", + "sas" + }; /** - * Matches {@code key:value} / {@code key=value} pairs whose key looks sensitive. The value runs - * up to a quote, comma, brace, ampersand or line break, so multi-token values such as {@code - * Authorization: Bearer } are masked whole rather than only their first word. + * Markers that flag free-form text (e.g. a server error message or a signed URL) as possibly + * carrying a secret. Matched by a plain lower-cased substring scan, so no regex backtracking. */ - private static final Pattern SENSITIVE_TEXT = - Pattern.compile( - "(?i)([\"']?[\\w.-]*" - + "(?:secret|token|password|credential|access[._-]?key|account[._-]?key" - + "|encryption[._-]?key|authorization|api[._-]?key|private[._-]?key|sas)" - + "[\\w.-]*[\"']?\\s*[:=]\\s*)([\"']?)([^\"',&}\\r\\n]+)"); + private static final String[] SENSITIVE_TEXT_MARKERS = { + "password", + "secret", + "token", + "credential", + "access-key", + "accesskey", + "account-key", + "accountkey", + "authorization", + "private-key", + "privatekey", + "api-key", + "apikey", + "encryption-key", + "encryptionkey", + "signature", + "sig=", + "x-amz-" + }; private SensitiveConfigUtils() {} @@ -119,23 +146,58 @@ public static Map redactMap(Map options) { } /** - * Best-effort masking of secret values in free-form text (JSON/form HTTP bodies): masks the - * value after any sensitive-looking key. Prefer {@link #redactMap} when keys are known. + * Redacts free-form text (e.g. a server {@code ErrorResponse.message}). Arbitrary text cannot + * be masked per-secret reliably, so if any sensitive marker is present the whole text is + * replaced. Runs a plain linear substring scan (no regex backtracking / ReDoS) and is safe to + * call on attacker-controlled input. Prefer {@link #redactMap} when the keys are known. */ public static String redactText(String text) { if (text == null || text.isEmpty()) { return text; } - Matcher matcher = SENSITIVE_TEXT.matcher(text); - StringBuffer sb = new StringBuffer(); - while (matcher.find()) { - boolean fullMask = matchesAny(matcher.group(1), FULL_MASK_KEY_PATTERNS); - String replacement = - matcher.group(1) + matcher.group(2) + maskValue(matcher.group(3), fullMask); - matcher.appendReplacement(sb, Matcher.quoteReplacement(replacement)); + String lower = text.toLowerCase(Locale.ROOT); + for (String marker : SENSITIVE_TEXT_MARKERS) { + if (lower.contains(marker)) { + return REDACTED; + } } - matcher.appendTail(sb); - return sb.toString(); + return text; + } + + /** + * Strips the query string and user-info from a URI so signed-URL credentials (AWS/GCS + * signature, Azure SAS {@code sig}, an embedded {@code user:password}) never reach logs or + * exceptions. Returns {@code scheme://host[:port]/path}; falls back to the substring before + * {@code '?'} when the URI cannot be parsed. + */ + public static String sanitizeUri(String uri) { + if (uri == null || uri.isEmpty()) { + return uri; + } + try { + URI parsed = new URI(uri); + StringBuilder sb = new StringBuilder(); + if (parsed.getScheme() != null) { + sb.append(parsed.getScheme()).append("://"); + } + if (parsed.getHost() != null) { + sb.append(parsed.getHost()); + if (parsed.getPort() != -1) { + sb.append(':').append(parsed.getPort()); + } + } + if (parsed.getRawPath() != null) { + sb.append(parsed.getRawPath()); + } + return sb.length() == 0 ? stripQuery(uri) : sb.toString(); + } catch (URISyntaxException e) { + return stripQuery(uri); + } + } + + private static String stripQuery(String uri) { + int queryStart = uri.indexOf('?'); + return queryStart >= 0 ? uri.substring(0, queryStart) : uri; } /** diff --git a/paimon-api/src/test/java/org/apache/paimon/utils/SensitiveConfigUtilsTest.java b/paimon-api/src/test/java/org/apache/paimon/utils/SensitiveConfigUtilsTest.java index 430a7e00b6f2..ebec39d07552 100644 --- a/paimon-api/src/test/java/org/apache/paimon/utils/SensitiveConfigUtilsTest.java +++ b/paimon-api/src/test/java/org/apache/paimon/utils/SensitiveConfigUtilsTest.java @@ -63,8 +63,8 @@ void testIsSensitiveIgnoresNonCredentialKeys() { @Test void testRedactValue() { - // Secret/access-key values keep only their last 4 chars. - assertThat(SensitiveConfigUtils.redactValue("fs.oss.accessKeySecret", "0123456789abcdef")) + // Identifier-like access-key id keeps only its last 4 chars. + assertThat(SensitiveConfigUtils.redactValue("fs.oss.accessKeyId", "0123456789abcdef")) .isEqualTo("****cdef"); // Short value is fully masked. assertThat(SensitiveConfigUtils.redactValue("password", "short")).isEqualTo(REDACTED); @@ -73,16 +73,27 @@ void testRedactValue() { } @Test - void testPasswordAndTokenAreFullyMasked() { - // Passwords and tokens leak too much from a trailing hint -> always fully masked. - assertThat(SensitiveConfigUtils.redactValue("my.password", "0123456789abcdef")) - .isEqualTo(REDACTED); - assertThat(SensitiveConfigUtils.redactValue("security.token", "0123456789abcdef")) - .isEqualTo(REDACTED); - assertThat(SensitiveConfigUtils.redactValue("Authorization", "Bearer 0123456789abcdef")) - .isEqualTo(REDACTED); - // Contrast: an access-key secret keeps its tail. - assertThat(SensitiveConfigUtils.redactValue("fs.s3a.secret.key", "0123456789abcdef")) + void testTrueSecretsAreFullyMasked() { + // True secrets are never partially revealed (AWS/Azure: the secret is never shown). + for (String key : + new String[] { + "my.password", + "security.token", + "Authorization", + "fs.s3a.secret.key", + "fs.oss.accessKeySecret", + "client.private-key", + "fs.s3a.encryption.key", + "fs.azure.account.key.acct", + "fs.azure.sas.acct", + "some.credential" + }) { + assertThat(SensitiveConfigUtils.redactValue(key, "0123456789abcdef")) + .as(key) + .isEqualTo(REDACTED); + } + // Contrast: an access-key id (an identifier) keeps its tail. + assertThat(SensitiveConfigUtils.redactValue("fs.oss.accessKeyId", "0123456789abcdef")) .isEqualTo("****cdef"); } @@ -98,10 +109,10 @@ void testRedactMapReplacesOnlySensitiveValues() { Map redacted = SensitiveConfigUtils.redactMap(options); assertThat(redacted).containsEntry("warehouse", "mock://warehouse"); - // Long secrets keep their last 4 chars; endpoint is untouched. + // Access-key id keeps a tail; true secrets are fully masked; endpoint is untouched. assertThat(redacted).containsEntry("fs.oss.accessKeyId", "****0001"); - assertThat(redacted).containsEntry("fs.oss.accessKeySecret", "****alue"); - assertThat(redacted).containsEntry("fs.azure.account.key.acct", "****alue"); + assertThat(redacted).containsEntry("fs.oss.accessKeySecret", REDACTED); + assertThat(redacted).containsEntry("fs.azure.account.key.acct", REDACTED); assertThat(redacted).containsEntry("fs.oss.endpoint", "mock-endpoint.example.com"); // Original map must not be mutated. assertThat(options).containsEntry("fs.oss.accessKeySecret", "mock-secret-value"); @@ -117,45 +128,28 @@ void testRedactMapNullSafe() { } @Test - void testRedactTextMasksSecretsInJsonAndForm() { - String json = - "{\"accessKeyId\":\"mock-id\",\"accessKeySecret\":\"mock-secret-abcd\"," - + "\"securityToken\":\"mock-tok\",\"endpoint\":\"mock-endpoint\"}"; - String redactedJson = SensitiveConfigUtils.redactText(json); - assertThat(redactedJson) - .doesNotContain("mock-secret-abcd") - .doesNotContain("mock-tok") - .contains(REDACTED) - // Non-sensitive fields are kept. - .contains("mock-endpoint"); - - String form = "password=mock-pass&user=alice&fs.oss.access-key=mock-ak"; - String redactedForm = SensitiveConfigUtils.redactText(form); - assertThat(redactedForm) - .doesNotContain("mock-pass") - .doesNotContain("mock-ak") - .contains("user=alice"); - } - - @Test - void testRedactTextMasksWholeValueWithSpaces() { - // The token after the "Bearer " scheme (with a space) must be masked, not just "Bearer". - String header = "Authorization: Bearer mock-jwt-token-abcdef"; - assertThat(SensitiveConfigUtils.redactText(header)) - .doesNotContain("mock-jwt-token-abcdef") - .startsWith("Authorization: "); - - // A quoted multi-word secret value is masked up to the closing quote only. - String json = "{\"token\":\"Bearer mock-jwt-abcd\",\"user\":\"alice\"}"; - assertThat(SensitiveConfigUtils.redactText(json)) - .doesNotContain("mock-jwt-abcd") - .contains("\"user\":\"alice\""); + void testRedactTextRedactsWholeMarkedText() { + // Free-form text cannot be masked per-secret reliably, so any marker redacts it whole. + // This covers the cases a boundary regex leaked: commas, spaces, quotes, Azure sig. + for (String text : + new String[] { + "{\"accessKeySecret\":\"mock-secret-abcd\",\"endpoint\":\"mock\"}", + "password=mock-pass&user=alice", + "Authorization: Bearer mock-jwt-token-abcdef", + "{\"password\":\"alpha,beta\"}", + "{\"sas\":\"sv=2024-11-04&sig=REST-LEAK\"}", + "invalid token SECRET-9999 in request", + "https://x.blob.core.windows.net/c/f?sig=SECRETSIG" + }) { + assertThat(SensitiveConfigUtils.redactText(text)).as(text).isEqualTo(REDACTED); + } } @Test - void testRedactTextNullAndEmptySafe() { + void testRedactTextKeepsTextWithoutMarkers() { assertThat(SensitiveConfigUtils.redactText(null)).isNull(); assertThat(SensitiveConfigUtils.redactText("")).isEmpty(); - assertThat(SensitiveConfigUtils.redactText("no secrets here")).isEqualTo("no secrets here"); + assertThat(SensitiveConfigUtils.redactText("Table not found: default.t")) + .isEqualTo("Table not found: default.t"); } } diff --git a/paimon-filesystems/paimon-cosn-impl/src/main/java/org/apache/paimon/cosn/COSNFileIO.java b/paimon-filesystems/paimon-cosn-impl/src/main/java/org/apache/paimon/cosn/COSNFileIO.java index 00b0bc60127d..0f0096751d9e 100644 --- a/paimon-filesystems/paimon-cosn-impl/src/main/java/org/apache/paimon/cosn/COSNFileIO.java +++ b/paimon-filesystems/paimon-cosn-impl/src/main/java/org/apache/paimon/cosn/COSNFileIO.java @@ -21,6 +21,7 @@ import org.apache.paimon.catalog.CatalogContext; import org.apache.paimon.fs.FileIO; import org.apache.paimon.options.Options; +import org.apache.paimon.utils.SensitiveConfigUtils; import org.apache.hadoop.conf.Configuration; import org.apache.hadoop.fs.FileSystem; @@ -83,7 +84,7 @@ public void configure(CatalogContext context) { LOG.debug( "Adding config entry for {} as {} to Hadoop config", key, - hadoopOptions.get(key)); + SensitiveConfigUtils.redactValue(key, hadoopOptions.get(key))); } } } diff --git a/paimon-filesystems/paimon-gs-impl/src/main/java/org/apache/paimon/gs/GSFileIO.java b/paimon-filesystems/paimon-gs-impl/src/main/java/org/apache/paimon/gs/GSFileIO.java index 2226d8be32bc..d2efcad70c7b 100644 --- a/paimon-filesystems/paimon-gs-impl/src/main/java/org/apache/paimon/gs/GSFileIO.java +++ b/paimon-filesystems/paimon-gs-impl/src/main/java/org/apache/paimon/gs/GSFileIO.java @@ -21,6 +21,7 @@ import org.apache.paimon.catalog.CatalogContext; import org.apache.paimon.fs.FileIO; import org.apache.paimon.options.Options; +import org.apache.paimon.utils.SensitiveConfigUtils; import com.google.cloud.hadoop.fs.gcs.GoogleHadoopFileSystem; import org.apache.hadoop.conf.Configuration; @@ -70,7 +71,7 @@ public void configure(CatalogContext context) { LOG.warn( "Adding config entry for {} as {} to Hadoop config", key, - hadoopOptions.get(key)); + SensitiveConfigUtils.redactValue(key, hadoopOptions.get(key))); } } } diff --git a/paimon-filesystems/paimon-jindo/src/main/java/org/apache/paimon/jindo/JindoFileIO.java b/paimon-filesystems/paimon-jindo/src/main/java/org/apache/paimon/jindo/JindoFileIO.java index c5e2d13a7707..7cab897e7654 100644 --- a/paimon-filesystems/paimon-jindo/src/main/java/org/apache/paimon/jindo/JindoFileIO.java +++ b/paimon-filesystems/paimon-jindo/src/main/java/org/apache/paimon/jindo/JindoFileIO.java @@ -26,6 +26,7 @@ import org.apache.paimon.options.Options; import org.apache.paimon.utils.IOUtils; import org.apache.paimon.utils.Pair; +import org.apache.paimon.utils.SensitiveConfigUtils; import org.apache.paimon.utils.StringUtils; import com.aliyun.jindodata.common.JindoHadoopSystem; @@ -113,7 +114,7 @@ public void configure(CatalogContext context) { LOG.debug( "Adding config entry for {} as {} to Hadoop config", key, - hadoopOptions.get(key)); + SensitiveConfigUtils.redactValue(key, hadoopOptions.get(key))); } } } diff --git a/paimon-filesystems/paimon-obs-impl/src/main/java/org/apache/paimon/obs/OBSFileIO.java b/paimon-filesystems/paimon-obs-impl/src/main/java/org/apache/paimon/obs/OBSFileIO.java index ae11dc2f9e7e..4f425ca59065 100644 --- a/paimon-filesystems/paimon-obs-impl/src/main/java/org/apache/paimon/obs/OBSFileIO.java +++ b/paimon-filesystems/paimon-obs-impl/src/main/java/org/apache/paimon/obs/OBSFileIO.java @@ -21,6 +21,7 @@ import org.apache.paimon.catalog.CatalogContext; import org.apache.paimon.fs.FileIO; import org.apache.paimon.options.Options; +import org.apache.paimon.utils.SensitiveConfigUtils; import org.apache.hadoop.conf.Configuration; import org.apache.hadoop.fs.FileSystem; @@ -99,7 +100,7 @@ public void configure(CatalogContext context) { LOG.debug( "Adding config entry for {} as {} to Hadoop config", key, - hadoopOptions.get(key)); + SensitiveConfigUtils.redactValue(key, hadoopOptions.get(key))); } } } diff --git a/paimon-filesystems/paimon-oss-impl/src/main/java/org/apache/paimon/oss/OSSFileIO.java b/paimon-filesystems/paimon-oss-impl/src/main/java/org/apache/paimon/oss/OSSFileIO.java index 410c460bb419..c397eca2f3de 100644 --- a/paimon-filesystems/paimon-oss-impl/src/main/java/org/apache/paimon/oss/OSSFileIO.java +++ b/paimon-filesystems/paimon-oss-impl/src/main/java/org/apache/paimon/oss/OSSFileIO.java @@ -26,6 +26,7 @@ import org.apache.paimon.options.Options; import org.apache.paimon.utils.IOUtils; import org.apache.paimon.utils.ReflectionUtils; +import org.apache.paimon.utils.SensitiveConfigUtils; import org.apache.paimon.utils.StringUtils; import com.aliyun.oss.OSSClient; @@ -145,7 +146,7 @@ public void configure(CatalogContext context) { LOG.debug( "Adding config entry for {} as {} to Hadoop config", key, - hadoopOptions.get(key)); + SensitiveConfigUtils.redactValue(key, hadoopOptions.get(key))); } } } diff --git a/paimon-flink/paimon-flink-common/src/main/java/org/apache/paimon/flink/AbstractFlinkTableFactory.java b/paimon-flink/paimon-flink-common/src/main/java/org/apache/paimon/flink/AbstractFlinkTableFactory.java index c13a1bb158e4..c3e55247c34f 100644 --- a/paimon-flink/paimon-flink-common/src/main/java/org/apache/paimon/flink/AbstractFlinkTableFactory.java +++ b/paimon-flink/paimon-flink-common/src/main/java/org/apache/paimon/flink/AbstractFlinkTableFactory.java @@ -37,6 +37,7 @@ import org.apache.paimon.table.FormatTable; import org.apache.paimon.table.Table; import org.apache.paimon.utils.Preconditions; +import org.apache.paimon.utils.SensitiveConfigUtils; import org.apache.flink.api.common.RuntimeExecutionMode; import org.apache.flink.configuration.ConfigOption; @@ -289,7 +290,7 @@ static Map getDynamicConfigOptions(DynamicTableFactory.Context c LOG.info( "Loading dynamic table options for {} in table config: {}", tableName, - optionsFromTableConfig); + SensitiveConfigUtils.redactMap(optionsFromTableConfig)); } return optionsFromTableConfig; } From 89b18fae818c3722930de104e7e61bfe1a8245f8 Mon Sep 17 00:00:00 2001 From: "xiaohongbo.xhb" Date: Sun, 19 Jul 2026 02:03:19 -0700 Subject: [PATCH 06/13] [api] Harden URI sanitization and redactText separators; fix error label More review follow-ups: - sanitizeUri now strips an embedded user:password@ on the parse-failure path too (previously only the query was removed), and LoggingInterceptor / RESTUtil no longer log or chain the raw URISyntaxException whose message echoes the full URL. - redactText normalizes the text before scanning, so separator variants (api_key, private.key, accessKey, X-Amz-Signature) are caught, plus literal markers for Azure SAS sig. - Sanitize the BLOB signed URL in BlobFormatWriter and FlinkRowWrapper failure logs so token/signature query params do not leak. - buildErrorResponse only labels a body "unparseable" when it truly failed to parse; a parsed ErrorResponse with an empty message is reported as such, and the embedded status matches the resolved code. --- .../org/apache/paimon/rest/HttpClient.java | 7 ++-- .../java/org/apache/paimon/rest/RESTUtil.java | 3 +- .../rest/interceptor/LoggingInterceptor.java | 3 +- .../paimon/utils/SensitiveConfigUtils.java | 32 ++++++++++++------- .../utils/SensitiveConfigUtilsTest.java | 19 +++++++++-- .../apache/paimon/rest/HttpClientTest.java | 7 ++-- .../apache/paimon/flink/FlinkRowWrapper.java | 9 +++--- .../paimon/format/blob/BlobFormatWriter.java | 4 ++- 8 files changed, 57 insertions(+), 27 deletions(-) diff --git a/paimon-api/src/main/java/org/apache/paimon/rest/HttpClient.java b/paimon-api/src/main/java/org/apache/paimon/rest/HttpClient.java index 73c7890b3868..e7bf1aa41158 100644 --- a/paimon-api/src/main/java/org/apache/paimon/rest/HttpClient.java +++ b/paimon-api/src/main/java/org/apache/paimon/rest/HttpClient.java @@ -245,9 +245,12 @@ private static ErrorResponse buildErrorResponse(ErrorResponse error, int errorCo if (error != null && error.getMessage() != null && !error.getMessage().isEmpty()) { // A parsed message may still embed secrets (e.g. "password=..."); redact it. message = SensitiveConfigUtils.redactText(error.getMessage()); + } else if (error == null) { + // The body could not be parsed; it is arbitrary and is not echoed (may hold secrets). + message = "Unparseable error response body (HTTP " + code + ")."; } else { - // The raw body is arbitrary and cannot be reliably sanitized; do not echo it. - message = "Unparseable error response body (HTTP " + errorCode + ")."; + // Parsed as an ErrorResponse but with no message. + message = "Empty error message (HTTP " + code + ")."; } return new ErrorResponse(resourceType, resourceName, message, code); } diff --git a/paimon-api/src/main/java/org/apache/paimon/rest/RESTUtil.java b/paimon-api/src/main/java/org/apache/paimon/rest/RESTUtil.java index d7ee78f511df..280a4aabb821 100644 --- a/paimon-api/src/main/java/org/apache/paimon/rest/RESTUtil.java +++ b/paimon-api/src/main/java/org/apache/paimon/rest/RESTUtil.java @@ -202,7 +202,8 @@ public static String buildRequestUrl(String url, Map queryParams url = builder.build().toString(); } } catch (URISyntaxException e) { - throw new RESTException(e, "build request URL failed."); + // cause / getMessage() echo the raw URL (may carry credentials); keep only the reason. + throw new RESTException("build request URL failed: %s", e.getReason()); } return url; diff --git a/paimon-api/src/main/java/org/apache/paimon/rest/interceptor/LoggingInterceptor.java b/paimon-api/src/main/java/org/apache/paimon/rest/interceptor/LoggingInterceptor.java index 51c31bb6f69c..75898455cd75 100644 --- a/paimon-api/src/main/java/org/apache/paimon/rest/interceptor/LoggingInterceptor.java +++ b/paimon-api/src/main/java/org/apache/paimon/rest/interceptor/LoggingInterceptor.java @@ -60,7 +60,8 @@ public void process( SensitiveConfigUtils.sanitizeUri(request.getUri().toString()), durationMs); } catch (URISyntaxException e) { - LOG.warn("Failed to log rest request: {}", e.getMessage()); + // e.getMessage() echoes the raw URI (may carry credentials); log only the reason. + LOG.warn("Failed to log rest request: {}", e.getReason()); } } } diff --git a/paimon-api/src/main/java/org/apache/paimon/utils/SensitiveConfigUtils.java b/paimon-api/src/main/java/org/apache/paimon/utils/SensitiveConfigUtils.java index e4dc33681363..3fe5b48ad76d 100644 --- a/paimon-api/src/main/java/org/apache/paimon/utils/SensitiveConfigUtils.java +++ b/paimon-api/src/main/java/org/apache/paimon/utils/SensitiveConfigUtils.java @@ -80,30 +80,30 @@ public class SensitiveConfigUtils { }; /** - * Markers that flag free-form text (e.g. a server error message or a signed URL) as possibly - * carrying a secret. Matched by a plain lower-cased substring scan, so no regex backtracking. + * Markers that flag free-form text (a server error message or a signed URL) as possibly + * carrying a secret. Matched after the text is normalized (lower-cased, separators removed), so + * {@code api_key}, {@code private.key}, {@code accessKey} and {@code X-Amz-Signature} all hit. */ private static final String[] SENSITIVE_TEXT_MARKERS = { "password", "secret", "token", "credential", - "access-key", "accesskey", - "account-key", "accountkey", + "encryptionkey", "authorization", - "private-key", "privatekey", - "api-key", "apikey", - "encryption-key", - "encryptionkey", "signature", - "sig=", - "x-amz-" + "sas" }; + /** + * Literal markers (Azure SAS {@code sig}) that would be ambiguous once separators are removed. + */ + private static final String[] SENSITIVE_TEXT_LITERAL_MARKERS = {"sig=", "\"sig\"", "'sig'"}; + private SensitiveConfigUtils() {} /** Returns whether the given configuration key is considered sensitive. */ @@ -156,11 +156,17 @@ public static String redactText(String text) { return text; } String lower = text.toLowerCase(Locale.ROOT); - for (String marker : SENSITIVE_TEXT_MARKERS) { + for (String marker : SENSITIVE_TEXT_LITERAL_MARKERS) { if (lower.contains(marker)) { return REDACTED; } } + String normalized = lower.replaceAll("[^a-z0-9]", ""); + for (String marker : SENSITIVE_TEXT_MARKERS) { + if (normalized.contains(marker)) { + return REDACTED; + } + } return text; } @@ -197,7 +203,9 @@ public static String sanitizeUri(String uri) { private static String stripQuery(String uri) { int queryStart = uri.indexOf('?'); - return queryStart >= 0 ? uri.substring(0, queryStart) : uri; + String base = queryStart >= 0 ? uri.substring(0, queryStart) : uri; + // A malformed URI may keep an embedded user:password@; drop it too. + return base.replaceAll("://[^/@]*@", "://"); } /** diff --git a/paimon-api/src/test/java/org/apache/paimon/utils/SensitiveConfigUtilsTest.java b/paimon-api/src/test/java/org/apache/paimon/utils/SensitiveConfigUtilsTest.java index ebec39d07552..e84efc8f5317 100644 --- a/paimon-api/src/test/java/org/apache/paimon/utils/SensitiveConfigUtilsTest.java +++ b/paimon-api/src/test/java/org/apache/paimon/utils/SensitiveConfigUtilsTest.java @@ -129,8 +129,6 @@ void testRedactMapNullSafe() { @Test void testRedactTextRedactsWholeMarkedText() { - // Free-form text cannot be masked per-secret reliably, so any marker redacts it whole. - // This covers the cases a boundary regex leaked: commas, spaces, quotes, Azure sig. for (String text : new String[] { "{\"accessKeySecret\":\"mock-secret-abcd\",\"endpoint\":\"mock\"}", @@ -139,7 +137,11 @@ void testRedactTextRedactsWholeMarkedText() { "{\"password\":\"alpha,beta\"}", "{\"sas\":\"sv=2024-11-04&sig=REST-LEAK\"}", "invalid token SECRET-9999 in request", - "https://x.blob.core.windows.net/c/f?sig=SECRETSIG" + "https://x.blob.core.windows.net/c/f?sig=SECRETSIG", + "api_key=mock", + "private.key: mock", + "fs.s3a.access.key=mock", + "{\"apiKey\":\"mock\"}" }) { assertThat(SensitiveConfigUtils.redactText(text)).as(text).isEqualTo(REDACTED); } @@ -152,4 +154,15 @@ void testRedactTextKeepsTextWithoutMarkers() { assertThat(SensitiveConfigUtils.redactText("Table not found: default.t")) .isEqualTo("Table not found: default.t"); } + + @Test + void testSanitizeUriDropsQueryAndUserInfo() { + assertThat(SensitiveConfigUtils.sanitizeUri("https://host:443/p?sig=x&X-Amz-Signature=y")) + .isEqualTo("https://host:443/p"); + assertThat(SensitiveConfigUtils.sanitizeUri("https://alice:secret@host/p?sig=x")) + .isEqualTo("https://host/p"); + assertThat(SensitiveConfigUtils.sanitizeUri("https://alice:secret@host/bad path?sig=x")) + .doesNotContain("secret") + .doesNotContain("sig="); + } } diff --git a/paimon-core/src/test/java/org/apache/paimon/rest/HttpClientTest.java b/paimon-core/src/test/java/org/apache/paimon/rest/HttpClientTest.java index dadfd409379d..d18f0e6a19fc 100644 --- a/paimon-core/src/test/java/org/apache/paimon/rest/HttpClientTest.java +++ b/paimon-core/src/test/java/org/apache/paimon/rest/HttpClientTest.java @@ -275,8 +275,8 @@ private Map getParameters(String path) { @Test public void testGetWithUnparsableJsonErrorResponse() { - // A JSON body that cannot be mapped to ErrorResponse must NOT be echoed (it may carry - // secrets); the exception carries only a generic message with the HTTP status. + // A JSON body that parses to an ErrorResponse with no message must NOT be echoed (it may + // carry secrets); it is reported as an empty message, not "unparseable". String jsonWithUppercaseFields = "{\"Message\":\"Your request is denied as lack of ssl protect.\"," + "\"Code\":\"InvalidProtocol.NeedSsl\"}"; @@ -291,7 +291,8 @@ public void testGetWithUnparsableJsonErrorResponse() { || e.getMessage().contains(jsonWithUppercaseFields), "Raw response body must not be echoed"); Assertions.assertTrue( - e.getMessage().contains("403"), "Message should carry the HTTP status"); + e.getMessage().contains("Empty error message"), + "Parsed-but-empty message must not be labelled unparseable"); } } diff --git a/paimon-flink/paimon-flink-common/src/main/java/org/apache/paimon/flink/FlinkRowWrapper.java b/paimon-flink/paimon-flink-common/src/main/java/org/apache/paimon/flink/FlinkRowWrapper.java index a6d900262b5e..5e587f9780ff 100644 --- a/paimon-flink/paimon-flink-common/src/main/java/org/apache/paimon/flink/FlinkRowWrapper.java +++ b/paimon-flink/paimon-flink-common/src/main/java/org/apache/paimon/flink/FlinkRowWrapper.java @@ -33,6 +33,7 @@ import org.apache.paimon.rest.HttpClientUtils; import org.apache.paimon.types.DataTypeRoot; import org.apache.paimon.types.RowKind; +import org.apache.paimon.utils.SensitiveConfigUtils; import org.apache.paimon.utils.UriReaderFactory; import org.apache.flink.table.data.DecimalData; @@ -240,7 +241,7 @@ private boolean descriptorFileExists(int pos, BlobDescriptor descriptor) { } LOG.warn( "Failed to check blob descriptor file {} for BLOB field at position {}.", - descriptor.uri(), + SensitiveConfigUtils.sanitizeUri(descriptor.uri()), pos, e); throw new RuntimeException(e); @@ -250,7 +251,7 @@ private boolean descriptorFileExists(int pos, BlobDescriptor descriptor) { } LOG.warn( "Failed to check blob descriptor file {} for BLOB field at position {}.", - descriptor.uri(), + SensitiveConfigUtils.sanitizeUri(descriptor.uri()), pos, e); throw e; @@ -261,12 +262,12 @@ private void logMissingDescriptor(int pos, BlobDescriptor descriptor) { if (isHttpUri(descriptor.uri())) { LOG.warn( "Blob descriptor file {} returned HTTP 404, returning NULL for BLOB field at position {}.", - descriptor.uri(), + SensitiveConfigUtils.sanitizeUri(descriptor.uri()), pos); } else { LOG.warn( "Blob descriptor file {} does not exist, returning NULL for BLOB field at position {}.", - descriptor.uri(), + SensitiveConfigUtils.sanitizeUri(descriptor.uri()), pos); } } diff --git a/paimon-format/src/main/java/org/apache/paimon/format/blob/BlobFormatWriter.java b/paimon-format/src/main/java/org/apache/paimon/format/blob/BlobFormatWriter.java index 9de5a169d1e2..212f6d8f4a28 100644 --- a/paimon-format/src/main/java/org/apache/paimon/format/blob/BlobFormatWriter.java +++ b/paimon-format/src/main/java/org/apache/paimon/format/blob/BlobFormatWriter.java @@ -37,6 +37,7 @@ import org.apache.paimon.types.RowType; import org.apache.paimon.utils.DeltaVarintCompressor; import org.apache.paimon.utils.LongArrayList; +import org.apache.paimon.utils.SensitiveConfigUtils; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -382,7 +383,8 @@ private void writeNullElement() throws IOException { private static String blobUri(@Nullable Blob blob) { if (blob instanceof BlobRef) { - return ((BlobRef) blob).toDescriptor().uri(); + // Sanitize: a signed URL carries token/signature in its query. + return SensitiveConfigUtils.sanitizeUri(((BlobRef) blob).toDescriptor().uri()); } return "unknown"; } From 441464b85a5940e179782c6ac56be50264d0abb1 Mon Sep 17 00:00:00 2001 From: "xiaohongbo.xhb" Date: Sun, 19 Jul 2026 04:24:28 -0700 Subject: [PATCH 07/13] [api] Fail closed on unparseable URIs and stop leaking them via exceptions sanitizeUri guessed at malformed URIs with a regex, which left user-info behind when the password contains '/' or '@'; it now returns a fixed placeholder on parse failure, and also when the host is unresolved but an '@' is present (credential fragments can land in other components). HttpGet/HttpHead construction in HttpClientUtils is wrapped so an invalid URI throws a safe IllegalArgumentException without the original cause/message (which echo the full raw URL); isInvalidUriException recognizes it so the blob write-NULL path is unchanged. --- .../apache/paimon/rest/HttpClientUtils.java | 34 ++++++++++++++++--- .../paimon/utils/SensitiveConfigUtils.java | 22 ++++++------ .../paimon/rest/HttpClientUtilsTest.java | 20 +++++++++++ .../utils/SensitiveConfigUtilsTest.java | 18 ++++++++-- 4 files changed, 76 insertions(+), 18 deletions(-) diff --git a/paimon-api/src/main/java/org/apache/paimon/rest/HttpClientUtils.java b/paimon-api/src/main/java/org/apache/paimon/rest/HttpClientUtils.java index d35b81e9d2ba..ed46848fdd88 100644 --- a/paimon-api/src/main/java/org/apache/paimon/rest/HttpClientUtils.java +++ b/paimon-api/src/main/java/org/apache/paimon/rest/HttpClientUtils.java @@ -86,8 +86,11 @@ private static HttpClientConnectionManager configureConnectionManager() { return connectionManagerBuilder.build(); } + /** Message prefix of the safe exception thrown for an unparseable URI. */ + public static final String INVALID_URI_MESSAGE_PREFIX = "Invalid HTTP URI: "; + public static InputStream getAsInputStream(String uri) throws IOException { - HttpGet httpGet = new HttpGet(uri); + HttpGet httpGet = newHttpGet(uri); CloseableHttpResponse response = DEFAULT_HTTP_CLIENT.execute(httpGet); int statusCode = response.getCode(); if (statusCode != HttpStatus.SC_OK) { @@ -140,7 +143,8 @@ public static boolean isInvalidUriException(Throwable throwable) { } if (current instanceof IllegalArgumentException && current.getMessage() != null - && current.getMessage().contains("Illegal character")) { + && (current.getMessage().contains("Illegal character") + || current.getMessage().startsWith(INVALID_URI_MESSAGE_PREFIX))) { return true; } current = current.getCause(); @@ -186,20 +190,42 @@ private static Integer parseStatusCodeSuffix(String statusText) { } private static int headStatusCode(String uri) throws IOException { - HttpHead httpHead = new HttpHead(uri); + HttpHead httpHead = newHttpHead(uri); try (CloseableHttpResponse response = DEFAULT_HTTP_CLIENT.execute(httpHead)) { return response.getCode(); } } private static int getRangeStatusCode(String uri) throws IOException { - HttpGet httpGet = new HttpGet(uri); + HttpGet httpGet = newHttpGet(uri); httpGet.addHeader("Range", "bytes=0-0"); try (CloseableHttpResponse response = DEFAULT_HTTP_CLIENT.execute(httpGet)) { return response.getCode(); } } + private static HttpGet newHttpGet(String uri) { + try { + return new HttpGet(uri); + } catch (RuntimeException e) { + throw invalidUri(uri); + } + } + + private static HttpHead newHttpHead(String uri) { + try { + return new HttpHead(uri); + } catch (RuntimeException e) { + throw invalidUri(uri); + } + } + + // No cause: the original exception echoes the raw URI, which may hold credentials. + private static IllegalArgumentException invalidUri(String uri) { + return new IllegalArgumentException( + INVALID_URI_MESSAGE_PREFIX + SensitiveConfigUtils.sanitizeUri(uri)); + } + private static RuntimeException httpError(int statusCode) { return new RuntimeException("HTTP error code: " + statusCode); } diff --git a/paimon-api/src/main/java/org/apache/paimon/utils/SensitiveConfigUtils.java b/paimon-api/src/main/java/org/apache/paimon/utils/SensitiveConfigUtils.java index 3fe5b48ad76d..b12489514eef 100644 --- a/paimon-api/src/main/java/org/apache/paimon/utils/SensitiveConfigUtils.java +++ b/paimon-api/src/main/java/org/apache/paimon/utils/SensitiveConfigUtils.java @@ -170,11 +170,14 @@ public static String redactText(String text) { return text; } + /** Placeholder for a URI that cannot be parsed and thus cannot be safely sanitized. */ + public static final String INVALID_URI = ""; + /** * Strips the query string and user-info from a URI so signed-URL credentials (AWS/GCS * signature, Azure SAS {@code sig}, an embedded {@code user:password}) never reach logs or - * exceptions. Returns {@code scheme://host[:port]/path}; falls back to the substring before - * {@code '?'} when the URI cannot be parsed. + * exceptions. Returns {@code scheme://host[:port]/path}. Fail-closed: an unparseable URI is + * replaced by {@link #INVALID_URI} instead of guessing at its structure. */ public static String sanitizeUri(String uri) { if (uri == null || uri.isEmpty()) { @@ -182,6 +185,10 @@ public static String sanitizeUri(String uri) { } try { URI parsed = new URI(uri); + // Host unresolved but '@' present: credentials may hide elsewhere -> fail closed. + if (parsed.getHost() == null && uri.indexOf('@') >= 0) { + return INVALID_URI; + } StringBuilder sb = new StringBuilder(); if (parsed.getScheme() != null) { sb.append(parsed.getScheme()).append("://"); @@ -195,19 +202,12 @@ public static String sanitizeUri(String uri) { if (parsed.getRawPath() != null) { sb.append(parsed.getRawPath()); } - return sb.length() == 0 ? stripQuery(uri) : sb.toString(); + return sb.length() == 0 ? INVALID_URI : sb.toString(); } catch (URISyntaxException e) { - return stripQuery(uri); + return INVALID_URI; } } - private static String stripQuery(String uri) { - int queryStart = uri.indexOf('?'); - String base = queryStart >= 0 ? uri.substring(0, queryStart) : uri; - // A malformed URI may keep an embedded user:password@; drop it too. - return base.replaceAll("://[^/@]*@", "://"); - } - /** * Masks a value. Password/token keys are fully masked; other secrets keep their last {@link * #TAIL_LEN} chars when long enough, to aid troubleshooting. diff --git a/paimon-api/src/test/java/org/apache/paimon/rest/HttpClientUtilsTest.java b/paimon-api/src/test/java/org/apache/paimon/rest/HttpClientUtilsTest.java index d92cb2b5013a..35efe82f5e89 100644 --- a/paimon-api/src/test/java/org/apache/paimon/rest/HttpClientUtilsTest.java +++ b/paimon-api/src/test/java/org/apache/paimon/rest/HttpClientUtilsTest.java @@ -21,6 +21,7 @@ import com.sun.net.httpserver.HttpExchange; import com.sun.net.httpserver.HttpHandler; import com.sun.net.httpserver.HttpServer; +import org.assertj.core.api.ThrowableAssert; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; @@ -178,6 +179,25 @@ public void testGetAsInputStreamThrowsForNotFound() { .hasMessage("HTTP error code: 404"); } + @Test + public void testInvalidUriExceptionDoesNotLeakCredentials() { + String uri = "https://alice:secret@host/bad path?sig=QUERY_SECRET"; + for (ThrowableAssert.ThrowingCallable call : + new ThrowableAssert.ThrowingCallable[] { + () -> HttpClientUtils.exists(uri), () -> HttpClientUtils.getAsInputStream(uri) + }) { + assertThatThrownBy(call) + .isInstanceOf(IllegalArgumentException.class) + .hasNoCause() + .matches(e -> HttpClientUtils.isInvalidUriException(e)) + .satisfies( + e -> { + assertThat(String.valueOf(e)).doesNotContain("secret"); + assertThat(String.valueOf(e)).doesNotContain("QUERY_SECRET"); + }); + } + } + @Test public void testGetAsInputStreamDoesNotLeakConnectionsOnRepeatedNotFound() throws Exception { registerHandler( diff --git a/paimon-api/src/test/java/org/apache/paimon/utils/SensitiveConfigUtilsTest.java b/paimon-api/src/test/java/org/apache/paimon/utils/SensitiveConfigUtilsTest.java index e84efc8f5317..6120a75e5774 100644 --- a/paimon-api/src/test/java/org/apache/paimon/utils/SensitiveConfigUtilsTest.java +++ b/paimon-api/src/test/java/org/apache/paimon/utils/SensitiveConfigUtilsTest.java @@ -161,8 +161,20 @@ void testSanitizeUriDropsQueryAndUserInfo() { .isEqualTo("https://host:443/p"); assertThat(SensitiveConfigUtils.sanitizeUri("https://alice:secret@host/p?sig=x")) .isEqualTo("https://host/p"); - assertThat(SensitiveConfigUtils.sanitizeUri("https://alice:secret@host/bad path?sig=x")) - .doesNotContain("secret") - .doesNotContain("sig="); + } + + @Test + void testSanitizeUriFailsClosedOnUnparseableUri() { + for (String uri : + new String[] { + "https://alice:secret@host/bad path?sig=x", + "https://alice:se/cret@host/bad path?sig=x", + "https://alice:se@cret@host/bad path?sig=x", + "https://alice:se/cret@host/p?sig=x" + }) { + assertThat(SensitiveConfigUtils.sanitizeUri(uri)) + .as(uri) + .isEqualTo(SensitiveConfigUtils.INVALID_URI); + } } } From 59a143420099db6fab0e92c3e320d64485c1168f Mon Sep 17 00:00:00 2001 From: "xiaohongbo.xhb" Date: Sun, 19 Jul 2026 04:55:11 -0700 Subject: [PATCH 08/13] [common] Do not leak the raw URI when UriReaderFactory cannot parse it UriReaderFactory.create calls URI.create(input) on the blob URI before it reaches HttpClientUtils. On a malformed signed URL that threw an IllegalArgumentException whose message and cause echo the full URL (token/signature). Parse it defensively and rethrow a safe exception with only the sanitized URI and no cause. --- .../org/apache/paimon/utils/UriReaderFactory.java | 12 +++++++++++- .../apache/paimon/utils/UriReaderFactoryTest.java | 14 ++++++++++++++ 2 files changed, 25 insertions(+), 1 deletion(-) diff --git a/paimon-common/src/main/java/org/apache/paimon/utils/UriReaderFactory.java b/paimon-common/src/main/java/org/apache/paimon/utils/UriReaderFactory.java index a2fd2d4ad356..39250951e214 100644 --- a/paimon-common/src/main/java/org/apache/paimon/utils/UriReaderFactory.java +++ b/paimon-common/src/main/java/org/apache/paimon/utils/UriReaderFactory.java @@ -44,11 +44,21 @@ public UriReaderFactory(CatalogContext context) { } public UriReader create(String input) { - URI uri = URI.create(input); + URI uri = parseUri(input); UriKey key = new UriKey(uri.getScheme(), uri.getAuthority()); return readers.computeIfAbsent(key, k -> newReader(k, uri)); } + private static URI parseUri(String input) { + try { + return URI.create(input); + } catch (IllegalArgumentException e) { + // Original exception/cause echoes the raw URI (may carry credentials); drop them. + throw new IllegalArgumentException( + "Invalid URI: " + SensitiveConfigUtils.sanitizeUri(input)); + } + } + public boolean exists(String input) throws IOException { UriReader reader = create(input); if (reader instanceof UriReader.FileUriReader) { diff --git a/paimon-common/src/test/java/org/apache/paimon/utils/UriReaderFactoryTest.java b/paimon-common/src/test/java/org/apache/paimon/utils/UriReaderFactoryTest.java index 3cf1cbaf3e5d..e0679cb74fcd 100644 --- a/paimon-common/src/test/java/org/apache/paimon/utils/UriReaderFactoryTest.java +++ b/paimon-common/src/test/java/org/apache/paimon/utils/UriReaderFactoryTest.java @@ -36,6 +36,7 @@ import java.nio.file.Files; import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; /** Test for {@link UriReaderFactory}. */ public class UriReaderFactoryTest { @@ -68,6 +69,19 @@ public void testCreateHttpUriReader() { assertThat(reader).isInstanceOf(HttpUriReader.class); } + @Test + public void testInvalidUriDoesNotLeakCredentials() { + assertThatThrownBy( + () -> factory.create("https://alice:secret@host/bad path?sig=QUERY_SECRET")) + .isInstanceOf(IllegalArgumentException.class) + .hasNoCause() + .satisfies( + e -> { + assertThat(String.valueOf(e)).doesNotContain("secret"); + assertThat(String.valueOf(e)).doesNotContain("QUERY_SECRET"); + }); + } + @Test public void testCreateHttpsUriReader() { UriReader reader = factory.create("https://example.com/file.txt"); From 5f3bf506246baed3059306c3d7d06ce405912cc5 Mon Sep 17 00:00:00 2001 From: "xiaohongbo.xhb" Date: Sun, 19 Jul 2026 05:15:35 -0700 Subject: [PATCH 09/13] Sanitize HTTP execute-stage exceptions and unify invalid-URI classification Redirect/protocol failures at the execute stage (e.g. "Circular redirect to ") echo the target URL, which for a signed BLOB URL is a credential. Wrap the execute boundary in HttpClientUtils so these become an IOException with no original message/cause, reporting only the sanitized request URI; also drop the leaking cause in HttpClient.exec and the leaking message in SimpleHttpClient. Move the invalid-URI exception factory into SensitiveConfigUtils so UriReaderFactory and HttpClientUtils share one message prefix, fixing isInvalidUriException misclassifying UriReaderFactory failures. Add circular-redirect and cross-module classification tests. --- .../org/apache/paimon/rest/HttpClient.java | 3 +- .../apache/paimon/rest/HttpClientUtils.java | 39 ++++++++++++------- .../apache/paimon/rest/SimpleHttpClient.java | 4 +- .../paimon/utils/SensitiveConfigUtils.java | 12 ++++++ .../paimon/rest/HttpClientUtilsTest.java | 33 ++++++++++++++++ .../apache/paimon/utils/UriReaderFactory.java | 4 +- 6 files changed, 74 insertions(+), 21 deletions(-) diff --git a/paimon-api/src/main/java/org/apache/paimon/rest/HttpClient.java b/paimon-api/src/main/java/org/apache/paimon/rest/HttpClient.java index e7bf1aa41158..3a55df7ac0cc 100644 --- a/paimon-api/src/main/java/org/apache/paimon/rest/HttpClient.java +++ b/paimon-api/src/main/java/org/apache/paimon/rest/HttpClient.java @@ -161,8 +161,9 @@ private T exec(HttpUriRequestBase request, Class res } }); } catch (IOException e) { + // No cause: a redirect/protocol error message can echo the target URL (a signed URL). throw new RESTException( - e, "Error occurred while processing %s request", request.getMethod()); + "Error occurred while processing %s request", request.getMethod()); } } diff --git a/paimon-api/src/main/java/org/apache/paimon/rest/HttpClientUtils.java b/paimon-api/src/main/java/org/apache/paimon/rest/HttpClientUtils.java index ed46848fdd88..58ea97570b24 100644 --- a/paimon-api/src/main/java/org/apache/paimon/rest/HttpClientUtils.java +++ b/paimon-api/src/main/java/org/apache/paimon/rest/HttpClientUtils.java @@ -33,6 +33,7 @@ import org.apache.hc.client5.http.io.HttpClientConnectionManager; import org.apache.hc.client5.http.ssl.DefaultClientTlsStrategy; import org.apache.hc.client5.http.ssl.HttpsSupport; +import org.apache.hc.core5.http.ClassicHttpRequest; import org.apache.hc.core5.http.HttpStatus; import org.apache.hc.core5.reactor.ssl.SSLBufferMode; import org.apache.hc.core5.ssl.SSLContexts; @@ -86,12 +87,9 @@ private static HttpClientConnectionManager configureConnectionManager() { return connectionManagerBuilder.build(); } - /** Message prefix of the safe exception thrown for an unparseable URI. */ - public static final String INVALID_URI_MESSAGE_PREFIX = "Invalid HTTP URI: "; - public static InputStream getAsInputStream(String uri) throws IOException { HttpGet httpGet = newHttpGet(uri); - CloseableHttpResponse response = DEFAULT_HTTP_CLIENT.execute(httpGet); + CloseableHttpResponse response = execute(httpGet, uri); int statusCode = response.getCode(); if (statusCode != HttpStatus.SC_OK) { try { @@ -144,7 +142,8 @@ public static boolean isInvalidUriException(Throwable throwable) { if (current instanceof IllegalArgumentException && current.getMessage() != null && (current.getMessage().contains("Illegal character") - || current.getMessage().startsWith(INVALID_URI_MESSAGE_PREFIX))) { + || current.getMessage() + .startsWith(SensitiveConfigUtils.INVALID_URI_MESSAGE_PREFIX))) { return true; } current = current.getCause(); @@ -191,7 +190,7 @@ private static Integer parseStatusCodeSuffix(String statusText) { private static int headStatusCode(String uri) throws IOException { HttpHead httpHead = newHttpHead(uri); - try (CloseableHttpResponse response = DEFAULT_HTTP_CLIENT.execute(httpHead)) { + try (CloseableHttpResponse response = execute(httpHead, uri)) { return response.getCode(); } } @@ -199,16 +198,32 @@ private static int headStatusCode(String uri) throws IOException { private static int getRangeStatusCode(String uri) throws IOException { HttpGet httpGet = newHttpGet(uri); httpGet.addHeader("Range", "bytes=0-0"); - try (CloseableHttpResponse response = DEFAULT_HTTP_CLIENT.execute(httpGet)) { + try (CloseableHttpResponse response = execute(httpGet, uri)) { return response.getCode(); } } + /** + * Executes a request, converting any execute-stage failure into an exception that carries + * neither the original message nor cause. Redirect and protocol errors (e.g. "Circular redirect + * to <Location>") echo the target URL, which for a signed URL is a credential; only the + * sanitized request URI is reported. + */ + private static CloseableHttpResponse execute(ClassicHttpRequest request, String uri) + throws IOException { + try { + return DEFAULT_HTTP_CLIENT.execute(request); + } catch (IOException | RuntimeException e) { + throw new IOException( + "HTTP request failed for uri: " + SensitiveConfigUtils.sanitizeUri(uri)); + } + } + private static HttpGet newHttpGet(String uri) { try { return new HttpGet(uri); } catch (RuntimeException e) { - throw invalidUri(uri); + throw SensitiveConfigUtils.invalidUri(uri); } } @@ -216,16 +231,10 @@ private static HttpHead newHttpHead(String uri) { try { return new HttpHead(uri); } catch (RuntimeException e) { - throw invalidUri(uri); + throw SensitiveConfigUtils.invalidUri(uri); } } - // No cause: the original exception echoes the raw URI, which may hold credentials. - private static IllegalArgumentException invalidUri(String uri) { - return new IllegalArgumentException( - INVALID_URI_MESSAGE_PREFIX + SensitiveConfigUtils.sanitizeUri(uri)); - } - private static RuntimeException httpError(int statusCode) { return new RuntimeException("HTTP error code: " + statusCode); } diff --git a/paimon-api/src/main/java/org/apache/paimon/rest/SimpleHttpClient.java b/paimon-api/src/main/java/org/apache/paimon/rest/SimpleHttpClient.java index 04aaf3bfea64..ddf7cdddf01c 100644 --- a/paimon-api/src/main/java/org/apache/paimon/rest/SimpleHttpClient.java +++ b/paimon-api/src/main/java/org/apache/paimon/rest/SimpleHttpClient.java @@ -101,8 +101,8 @@ private String exec(HttpUriRequestBase request) { return responseBodyStr; }); } catch (IOException e) { - throw new RuntimeException( - "Failed to convert HTTP response body to string, error : " + e.getMessage()); + // No message from e: a redirect/protocol error can echo the target URL (a signed URL). + throw new RuntimeException("Failed to convert HTTP response body to string."); } } diff --git a/paimon-api/src/main/java/org/apache/paimon/utils/SensitiveConfigUtils.java b/paimon-api/src/main/java/org/apache/paimon/utils/SensitiveConfigUtils.java index b12489514eef..08d0338ba7f7 100644 --- a/paimon-api/src/main/java/org/apache/paimon/utils/SensitiveConfigUtils.java +++ b/paimon-api/src/main/java/org/apache/paimon/utils/SensitiveConfigUtils.java @@ -173,6 +173,18 @@ public static String redactText(String text) { /** Placeholder for a URI that cannot be parsed and thus cannot be safely sanitized. */ public static final String INVALID_URI = ""; + /** Message prefix of the safe exception thrown for an unparseable URI. */ + public static final String INVALID_URI_MESSAGE_PREFIX = "Invalid URI: "; + + /** + * Builds a safe exception for an unparseable URI. The original exception echoes the raw URI + * (which may carry credentials), so it is neither kept as a cause nor reused as the message. + * Callers across modules share this prefix so the exception can be classified uniformly. + */ + public static IllegalArgumentException invalidUri(String uri) { + return new IllegalArgumentException(INVALID_URI_MESSAGE_PREFIX + sanitizeUri(uri)); + } + /** * Strips the query string and user-info from a URI so signed-URL credentials (AWS/GCS * signature, Azure SAS {@code sig}, an embedded {@code user:password}) never reach logs or diff --git a/paimon-api/src/test/java/org/apache/paimon/rest/HttpClientUtilsTest.java b/paimon-api/src/test/java/org/apache/paimon/rest/HttpClientUtilsTest.java index 35efe82f5e89..cb56cc1faeca 100644 --- a/paimon-api/src/test/java/org/apache/paimon/rest/HttpClientUtilsTest.java +++ b/paimon-api/src/test/java/org/apache/paimon/rest/HttpClientUtilsTest.java @@ -18,6 +18,8 @@ package org.apache.paimon.rest; +import org.apache.paimon.utils.SensitiveConfigUtils; + import com.sun.net.httpserver.HttpExchange; import com.sun.net.httpserver.HttpHandler; import com.sun.net.httpserver.HttpServer; @@ -198,6 +200,32 @@ public void testInvalidUriExceptionDoesNotLeakCredentials() { } } + @Test + public void testExecuteFailureDoesNotLeakRedirectLocation() { + String secretLocation = url("/redirect") + "?sig=REDIRECT_SECRET"; + registerHandler( + "/redirect", + exchange -> { + exchange.getResponseHeaders().add("Location", secretLocation); + respond(exchange, 302, new byte[0]); + }); + + for (ThrowableAssert.ThrowingCallable call : + new ThrowableAssert.ThrowingCallable[] { + () -> HttpClientUtils.getAsInputStream(url("/redirect")), + () -> HttpClientUtils.exists(url("/redirect")) + }) { + assertThatThrownBy(call) + .isInstanceOf(IOException.class) + .hasNoCause() + .satisfies( + e -> { + assertThat(String.valueOf(e)).doesNotContain("REDIRECT_SECRET"); + assertThat(e.getMessage()).doesNotContain("sig="); + }); + } + } + @Test public void testGetAsInputStreamDoesNotLeakConnectionsOnRepeatedNotFound() throws Exception { registerHandler( @@ -244,6 +272,11 @@ public void testIsInvalidUriException() { HttpClientUtils.isInvalidUriException( new IllegalArgumentException("Illegal character in path"))) .isTrue(); + // The shared invalid-URI exception must classify uniformly across modules. + assertThat( + HttpClientUtils.isInvalidUriException( + SensitiveConfigUtils.invalidUri("https://host/bad path"))) + .isTrue(); assertThat( HttpClientUtils.isInvalidUriException( new RuntimeException("HTTP error code: 404"))) diff --git a/paimon-common/src/main/java/org/apache/paimon/utils/UriReaderFactory.java b/paimon-common/src/main/java/org/apache/paimon/utils/UriReaderFactory.java index 39250951e214..08f0691303e6 100644 --- a/paimon-common/src/main/java/org/apache/paimon/utils/UriReaderFactory.java +++ b/paimon-common/src/main/java/org/apache/paimon/utils/UriReaderFactory.java @@ -53,9 +53,7 @@ private static URI parseUri(String input) { try { return URI.create(input); } catch (IllegalArgumentException e) { - // Original exception/cause echoes the raw URI (may carry credentials); drop them. - throw new IllegalArgumentException( - "Invalid URI: " + SensitiveConfigUtils.sanitizeUri(input)); + throw SensitiveConfigUtils.invalidUri(input); } } From 13b668e5b5b499e751dace21cd01cdb1e04cc558 Mon Sep 17 00:00:00 2001 From: "xiaohongbo.xhb" Date: Sun, 19 Jul 2026 05:56:08 -0700 Subject: [PATCH 10/13] Update FlinkRowWrapperTest for sanitized invalid-URI message descriptorFileExists now surfaces the sanitized "Invalid URI: " instead of the raw "Illegal character in ...: " that echoed the URI. Assert the sanitized prefix and that the raw path is not leaked. --- .../test/java/org/apache/paimon/flink/FlinkRowWrapperTest.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/paimon-flink/paimon-flink-common/src/test/java/org/apache/paimon/flink/FlinkRowWrapperTest.java b/paimon-flink/paimon-flink-common/src/test/java/org/apache/paimon/flink/FlinkRowWrapperTest.java index 517917e4c011..3141ef706242 100644 --- a/paimon-flink/paimon-flink-common/src/test/java/org/apache/paimon/flink/FlinkRowWrapperTest.java +++ b/paimon-flink/paimon-flink-common/src/test/java/org/apache/paimon/flink/FlinkRowWrapperTest.java @@ -167,7 +167,8 @@ public void testInvalidUriThrowsWhenFetchFailureDisabled() { assertThatThrownBy(() -> wrapper.isNullAt(0)) .isInstanceOf(IllegalArgumentException.class) - .hasMessageContaining("Illegal character"); + .hasMessageContaining("Invalid URI") + .hasMessageNotContaining("1304008055350781673"); } @Test From e4a03b58e4f045e688b9523e85f7029f17789327 Mon Sep 17 00:00:00 2001 From: "xiaohongbo.xhb" Date: Sun, 19 Jul 2026 06:07:06 -0700 Subject: [PATCH 11/13] Sanitize HTTP request construction so a malformed URL never leaks the raw URL SimpleHttpClient and HttpClient built the request (new HttpGet/HttpPost/HttpDelete) before entering the safe exec()/execute() boundary, so a malformed URL threw an IllegalArgumentException from the constructor that echoed the full raw URL (query, user-info, signature). Route all construction through HttpClientUtils safe factories that convert a construction-stage failure into SensitiveConfigUtils.invalidUri(url). Add malformed GET/POST coverage in HttpClientTest and a new SimpleHttpClientTest. --- .../org/apache/paimon/rest/HttpClient.java | 8 +-- .../apache/paimon/rest/HttpClientUtils.java | 28 +++++++--- .../apache/paimon/rest/SimpleHttpClient.java | 4 +- .../paimon/rest/SimpleHttpClientTest.java | 55 +++++++++++++++++++ .../apache/paimon/rest/HttpClientTest.java | 30 ++++++++++ 5 files changed, 111 insertions(+), 14 deletions(-) create mode 100644 paimon-api/src/test/java/org/apache/paimon/rest/SimpleHttpClientTest.java diff --git a/paimon-api/src/main/java/org/apache/paimon/rest/HttpClient.java b/paimon-api/src/main/java/org/apache/paimon/rest/HttpClient.java index 3a55df7ac0cc..311a1a2d4abe 100644 --- a/paimon-api/src/main/java/org/apache/paimon/rest/HttpClient.java +++ b/paimon-api/src/main/java/org/apache/paimon/rest/HttpClient.java @@ -64,7 +64,7 @@ public HttpClient(String uri) { public T get( String path, Class responseType, RESTAuthFunction restAuthFunction) { Header[] authHeaders = getHeaders(path, "GET", "", restAuthFunction); - HttpGet httpGet = new HttpGet(getRequestUrl(path, null)); + HttpGet httpGet = HttpClientUtils.newHttpGet(getRequestUrl(path, null)); httpGet.setHeaders(authHeaders); return exec(httpGet, responseType); } @@ -76,7 +76,7 @@ public T get( Class responseType, RESTAuthFunction restAuthFunction) { Header[] authHeaders = getHeaders(path, queryParams, "GET", "", restAuthFunction); - HttpGet httpGet = new HttpGet(getRequestUrl(path, queryParams)); + HttpGet httpGet = HttpClientUtils.newHttpGet(getRequestUrl(path, queryParams)); httpGet.setHeaders(authHeaders); return exec(httpGet, responseType); } @@ -93,7 +93,7 @@ public T post( RESTRequest body, Class responseType, RESTAuthFunction restAuthFunction) { - HttpPost httpPost = new HttpPost(getRequestUrl(path, null)); + HttpPost httpPost = HttpClientUtils.newHttpPost(getRequestUrl(path, null)); String encodedBody = RESTUtil.encodedBody(body); if (encodedBody != null) { httpPost.setEntity(new StringEntity(encodedBody, ContentType.APPLICATION_JSON)); @@ -111,7 +111,7 @@ public T delete(String path, RESTAuthFunction restAuthF @Override public T delete( String path, RESTRequest body, RESTAuthFunction restAuthFunction) { - HttpDelete httpDelete = new HttpDelete(getRequestUrl(path, null)); + HttpDelete httpDelete = HttpClientUtils.newHttpDelete(getRequestUrl(path, null)); String encodedBody = RESTUtil.encodedBody(body); if (encodedBody != null) { httpDelete.setEntity(new StringEntity(encodedBody, ContentType.APPLICATION_JSON)); diff --git a/paimon-api/src/main/java/org/apache/paimon/rest/HttpClientUtils.java b/paimon-api/src/main/java/org/apache/paimon/rest/HttpClientUtils.java index 58ea97570b24..e4334e60e59d 100644 --- a/paimon-api/src/main/java/org/apache/paimon/rest/HttpClientUtils.java +++ b/paimon-api/src/main/java/org/apache/paimon/rest/HttpClientUtils.java @@ -22,8 +22,10 @@ import org.apache.paimon.rest.interceptor.TimingInterceptor; import org.apache.paimon.utils.SensitiveConfigUtils; +import org.apache.hc.client5.http.classic.methods.HttpDelete; import org.apache.hc.client5.http.classic.methods.HttpGet; import org.apache.hc.client5.http.classic.methods.HttpHead; +import org.apache.hc.client5.http.classic.methods.HttpPost; import org.apache.hc.client5.http.config.RequestConfig; import org.apache.hc.client5.http.impl.classic.CloseableHttpClient; import org.apache.hc.client5.http.impl.classic.CloseableHttpResponse; @@ -41,6 +43,7 @@ import java.io.IOException; import java.io.InputStream; +import java.util.function.Function; /** Utils for {@link HttpClientBuilder}. */ public class HttpClientUtils { @@ -219,17 +222,26 @@ private static CloseableHttpResponse execute(ClassicHttpRequest request, String } } - private static HttpGet newHttpGet(String uri) { - try { - return new HttpGet(uri); - } catch (RuntimeException e) { - throw SensitiveConfigUtils.invalidUri(uri); - } + public static HttpGet newHttpGet(String uri) { + return newRequest(uri, HttpGet::new); + } + + public static HttpHead newHttpHead(String uri) { + return newRequest(uri, HttpHead::new); + } + + public static HttpPost newHttpPost(String uri) { + return newRequest(uri, HttpPost::new); + } + + public static HttpDelete newHttpDelete(String uri) { + return newRequest(uri, HttpDelete::new); } - private static HttpHead newHttpHead(String uri) { + /** A malformed URL leaks the raw URL from the constructor; sanitize it. */ + private static T newRequest(String uri, Function constructor) { try { - return new HttpHead(uri); + return constructor.apply(uri); } catch (RuntimeException e) { throw SensitiveConfigUtils.invalidUri(uri); } diff --git a/paimon-api/src/main/java/org/apache/paimon/rest/SimpleHttpClient.java b/paimon-api/src/main/java/org/apache/paimon/rest/SimpleHttpClient.java index ddf7cdddf01c..1fc21a31eb89 100644 --- a/paimon-api/src/main/java/org/apache/paimon/rest/SimpleHttpClient.java +++ b/paimon-api/src/main/java/org/apache/paimon/rest/SimpleHttpClient.java @@ -47,7 +47,7 @@ private SimpleHttpClient() { } public String post(String url, Object body, Map headers) throws IOException { - HttpPost httpPost = new HttpPost(url); + HttpPost httpPost = HttpClientUtils.newHttpPost(url); if (headers != null) { httpPost.setHeaders( headers.entrySet().stream() @@ -68,7 +68,7 @@ public String post(String url, Object body, Map headers) throws public String get(String url, Map queryParams, Map headers) throws IOException { - HttpGet httpGet = new HttpGet(RESTUtil.buildRequestUrl(url, queryParams)); + HttpGet httpGet = HttpClientUtils.newHttpGet(RESTUtil.buildRequestUrl(url, queryParams)); if (headers != null) { httpGet.setHeaders( headers.entrySet().stream() diff --git a/paimon-api/src/test/java/org/apache/paimon/rest/SimpleHttpClientTest.java b/paimon-api/src/test/java/org/apache/paimon/rest/SimpleHttpClientTest.java new file mode 100644 index 000000000000..0f426918c579 --- /dev/null +++ b/paimon-api/src/test/java/org/apache/paimon/rest/SimpleHttpClientTest.java @@ -0,0 +1,55 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.paimon.rest; + +import org.junit.jupiter.api.Test; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; + +/** Tests for {@link SimpleHttpClient}. */ +public class SimpleHttpClientTest { + + private static final String MALFORMED_URL = + "https://alice:secret@host/bad path?sig=QUERY_SECRET"; + + @Test + public void testGetWithMalformedUrlDoesNotLeakCredentials() { + assertMalformedUrlSanitized(() -> SimpleHttpClient.INSTANCE.get(MALFORMED_URL)); + } + + @Test + public void testPostWithMalformedUrlDoesNotLeakCredentials() { + assertMalformedUrlSanitized( + () -> SimpleHttpClient.INSTANCE.post(MALFORMED_URL, null, null)); + } + + private static void assertMalformedUrlSanitized( + org.assertj.core.api.ThrowableAssert.ThrowingCallable call) { + assertThatThrownBy(call) + .isInstanceOf(IllegalArgumentException.class) + .hasNoCause() + .matches(HttpClientUtils::isInvalidUriException) + .satisfies( + e -> { + assertThat(String.valueOf(e)).doesNotContain("secret"); + assertThat(String.valueOf(e)).doesNotContain("QUERY_SECRET"); + }); + } +} diff --git a/paimon-core/src/test/java/org/apache/paimon/rest/HttpClientTest.java b/paimon-core/src/test/java/org/apache/paimon/rest/HttpClientTest.java index d18f0e6a19fc..5bdc553fdf6a 100644 --- a/paimon-core/src/test/java/org/apache/paimon/rest/HttpClientTest.java +++ b/paimon-core/src/test/java/org/apache/paimon/rest/HttpClientTest.java @@ -315,6 +315,36 @@ public void testPostWithNonJsonErrorResponse() { } } + @Test + public void testGetWithMalformedUrlDoesNotLeakCredentials() { + // Malformed URL throws in the constructor before exec(); the raw URL must not leak. + String secret = "QUERY_SECRET"; + IllegalArgumentException e = + assertThrows( + IllegalArgumentException.class, + () -> + httpClient.get( + "/x?sig=" + secret + " bad", + MockRESTData.class, + restAuthFunction)); + assertFalse(e.getMessage().contains(secret)); + } + + @Test + public void testPostWithMalformedUrlDoesNotLeakCredentials() { + String secret = "QUERY_SECRET"; + IllegalArgumentException e = + assertThrows( + IllegalArgumentException.class, + () -> + httpClient.post( + "/x?sig=" + secret + " bad", + mockResponseData, + MockRESTData.class, + restAuthFunction)); + assertFalse(e.getMessage().contains(secret)); + } + @Test public void testPostSetsJsonContentType() throws Exception { server.enqueueResponse(mockResponseDataStr, 200); From c2bc63bc3fd338ec42f490facd07053767d472be Mon Sep 17 00:00:00 2001 From: "xiaohongbo.xhb" Date: Sun, 19 Jul 2026 06:26:20 -0700 Subject: [PATCH 12/13] Do not leak ECS token credentials on malformed JSON A malformed ECS metadata token response makes RESTApi.fromJson throw a JsonProcessingException (which extends IOException) whose message embeds the raw token JSON (AccessKeyId/AccessKeySecret/SecurityToken). It was wrapped keeping the message and cause, leaking the credentials. Parse in a separate try that reports a generic message with no body, snippet, or cause. --- .../paimon/rest/auth/DLFECSTokenLoader.java | 14 +++- .../rest/auth/DLFECSTokenLoaderTest.java | 80 +++++++++++++++++++ 2 files changed, 90 insertions(+), 4 deletions(-) create mode 100644 paimon-api/src/test/java/org/apache/paimon/rest/auth/DLFECSTokenLoaderTest.java diff --git a/paimon-api/src/main/java/org/apache/paimon/rest/auth/DLFECSTokenLoader.java b/paimon-api/src/main/java/org/apache/paimon/rest/auth/DLFECSTokenLoader.java index 404cb49b8db2..56b687ee12f4 100644 --- a/paimon-api/src/main/java/org/apache/paimon/rest/auth/DLFECSTokenLoader.java +++ b/paimon-api/src/main/java/org/apache/paimon/rest/auth/DLFECSTokenLoader.java @@ -22,6 +22,8 @@ import org.apache.paimon.rest.RESTApi; import org.apache.paimon.rest.SimpleHttpClient; +import org.apache.paimon.shade.jackson2.com.fasterxml.jackson.core.JsonProcessingException; + import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -66,13 +68,17 @@ private static String getRole(String url) { } private static DLFToken getToken(String url) { + String token; try { - String token = SimpleHttpClient.INSTANCE.get(url); - return RESTApi.fromJson(token, DLFToken.class); + token = SimpleHttpClient.INSTANCE.get(url); } catch (IOException e) { throw new UncheckedIOException(e); - } catch (Exception e) { - throw new RuntimeException("get token failed, error : " + e.getMessage(), e); + } + try { + return RESTApi.fromJson(token, DLFToken.class); + } catch (JsonProcessingException e) { + // The token response carries AK/SK/STS; never surface the body or Jackson's snippet. + throw new RuntimeException("Failed to parse ECS token response."); } } diff --git a/paimon-api/src/test/java/org/apache/paimon/rest/auth/DLFECSTokenLoaderTest.java b/paimon-api/src/test/java/org/apache/paimon/rest/auth/DLFECSTokenLoaderTest.java new file mode 100644 index 000000000000..704ff8a4501f --- /dev/null +++ b/paimon-api/src/test/java/org/apache/paimon/rest/auth/DLFECSTokenLoaderTest.java @@ -0,0 +1,80 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.paimon.rest.auth; + +import com.sun.net.httpserver.HttpExchange; +import com.sun.net.httpserver.HttpServer; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; + +import java.io.IOException; +import java.io.OutputStream; +import java.net.InetSocketAddress; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; + +/** Tests for {@link DLFECSTokenLoader}. */ +public class DLFECSTokenLoaderTest { + + private HttpServer server; + private int port; + + @BeforeEach + public void setUp() throws Exception { + server = HttpServer.create(new InetSocketAddress(0), 0); + port = server.getAddress().getPort(); + server.start(); + } + + @AfterEach + public void tearDown() { + if (server != null) { + server.stop(0); + } + } + + @Test + public void testMalformedTokenResponseDoesNotLeakCredentials() { + String secret = "STSSECRET_AKID_9999"; + server.createContext( + "/token", + exchange -> + respond( + exchange, + ("{\"AccessKeyId\":\"akid\",\"AccessKeySecret\":\"" + + secret + + "\" INVALID_JSON") + .getBytes())); + + DLFECSTokenLoader loader = new DLFECSTokenLoader("http://127.0.0.1:" + port + "/token", ""); + + assertThatThrownBy(loader::loadToken) + .hasNoCause() + .satisfies(e -> assertThat(String.valueOf(e)).doesNotContain(secret)); + } + + private static void respond(HttpExchange exchange, byte[] body) throws IOException { + exchange.sendResponseHeaders(200, body.length); + try (OutputStream out = exchange.getResponseBody()) { + out.write(body); + } + } +} From fb0e8af17a523b8bdb7a9e19ef6274434064db28 Mon Sep 17 00:00:00 2001 From: "xiaohongbo.xhb" Date: Sun, 19 Jul 2026 06:39:06 -0700 Subject: [PATCH 13/13] Do not leak local-file token credentials on malformed JSON DLFLocalFileTokenLoader had the same leak as the ECS loader: a malformed token file makes RESTApi.fromJson throw a JsonProcessingException embedding the raw token JSON (AK/SK/STS), which was kept as the cause of the final RuntimeException. Catch the parse failure separately and report a generic message with no body or cause; retry is preserved. Add a malformed-token-file test via a test-only retry-count seam. --- .../rest/auth/DLFLocalFileTokenLoader.java | 20 ++++++-- .../auth/DLFLocalFileTokenLoaderTest.java | 48 +++++++++++++++++++ 2 files changed, 64 insertions(+), 4 deletions(-) create mode 100644 paimon-api/src/test/java/org/apache/paimon/rest/auth/DLFLocalFileTokenLoaderTest.java diff --git a/paimon-api/src/main/java/org/apache/paimon/rest/auth/DLFLocalFileTokenLoader.java b/paimon-api/src/main/java/org/apache/paimon/rest/auth/DLFLocalFileTokenLoader.java index 199152474426..5514cd34c0bf 100644 --- a/paimon-api/src/main/java/org/apache/paimon/rest/auth/DLFLocalFileTokenLoader.java +++ b/paimon-api/src/main/java/org/apache/paimon/rest/auth/DLFLocalFileTokenLoader.java @@ -18,9 +18,12 @@ package org.apache.paimon.rest.auth; +import org.apache.paimon.annotation.VisibleForTesting; import org.apache.paimon.rest.RESTApi; import org.apache.paimon.utils.FileReadUtils; +import org.apache.paimon.shade.jackson2.com.fasterxml.jackson.core.JsonProcessingException; + import java.io.File; /** DLF Token Loader for local file. */ @@ -43,14 +46,23 @@ public String description() { } protected static DLFToken readToken(String tokenFilePath) { + return readToken(tokenFilePath, 5); + } + + @VisibleForTesting + static DLFToken readToken(String tokenFilePath, int maxRetries) { int retry = 1; - Exception lastException = null; - while (retry <= 5) { + RuntimeException lastException = null; + while (retry <= maxRetries) { try { String tokenStr = FileReadUtils.readFileUtf8(new File(tokenFilePath)); return RESTApi.fromJson(tokenStr, DLFToken.class); + } catch (JsonProcessingException e) { + // The token file carries AK/SK/STS; never keep the body or Jackson's snippet. + lastException = new RuntimeException("Failed to parse token file."); } catch (Exception e) { - lastException = e; + lastException = + new RuntimeException("Failed to read token file: " + tokenFilePath, e); } try { Thread.sleep(retry * 1000L); @@ -60,6 +72,6 @@ protected static DLFToken readToken(String tokenFilePath) { } retry++; } - throw new RuntimeException(lastException); + throw lastException; } } diff --git a/paimon-api/src/test/java/org/apache/paimon/rest/auth/DLFLocalFileTokenLoaderTest.java b/paimon-api/src/test/java/org/apache/paimon/rest/auth/DLFLocalFileTokenLoaderTest.java new file mode 100644 index 000000000000..b56ab4e2db59 --- /dev/null +++ b/paimon-api/src/test/java/org/apache/paimon/rest/auth/DLFLocalFileTokenLoaderTest.java @@ -0,0 +1,48 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.paimon.rest.auth; + +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.io.TempDir; + +import java.nio.file.Files; +import java.nio.file.Path; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; + +/** Tests for {@link DLFLocalFileTokenLoader}. */ +public class DLFLocalFileTokenLoaderTest { + + @TempDir Path tempDir; + + @Test + public void testMalformedTokenFileDoesNotLeakCredentials() throws Exception { + String secret = "STSSECRET_AKID_9999"; + Path tokenFile = tempDir.resolve("token.json"); + Files.write( + tokenFile, + ("{\"AccessKeyId\":\"akid\",\"AccessKeySecret\":\"" + secret + "\" INVALID_JSON") + .getBytes()); + + assertThatThrownBy(() -> DLFLocalFileTokenLoader.readToken(tokenFile.toString(), 1)) + .hasNoCause() + .satisfies(e -> assertThat(String.valueOf(e)).doesNotContain(secret)); + } +}