lasdf1234 commented on code in PR #13858:
URL: https://github.com/apache/gravitino/pull/13858#discussion_r4290882993


##########
core/src/main/java/org/apache/gravitino/job/JobRuntimeTemplateMaskUtils.java:
##########
@@ -0,0 +1,440 @@
+/*
+ * 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.gravitino.job;
+
+import com.google.common.collect.ImmutableSet;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+import javax.annotation.Nullable;
+import org.apache.gravitino.connector.HiddenPropertyMaskUtils;
+import org.apache.gravitino.dto.job.JobTemplateDTO;
+import org.apache.gravitino.dto.job.ShellJobTemplateDTO;
+import org.apache.gravitino.dto.job.SparkJobTemplateDTO;
+import org.apache.gravitino.secret.SecretPropertyUtils;
+
+/**
+ * Masks sensitive-named values on resolved runtime job templates before they 
are exposed on API or
+ * event paths. Stored runtime JSON is unchanged.
+ *
+ * <p>Argument masking is grouped by payload shape:
+ *
+ * <ul>
+ *   <li>Nest JSON — {@link #NESTED_JSON_ARGUMENT_FLAGS} ({@code 
--updater-options} / {@code
+ *       --spark-conf} / {@code --options})
+ *   <li>Spark conf property — {@link #CONF_ARGUMENT_FLAGS} ({@code --conf} / 
{@code -c}): a single
+ *       {@code key=value} in either two-token or {@code --conf=key=value} form
+ *   <li>JVM options string — {@link #JVM_OPTIONS_ARGUMENT_FLAGS} ({@code 
--driver-java-options} /
+ *       {@code --executor-java-options}): a space-separated JVM string that 
may start with {@code
+ *       -}; sensitive {@code -Dkey=value} tokens inside are masked
+ *   <li>Sensitive-named long options — {@code --name} / {@code --name=value} 
when the name matches
+ *       {@link SecretPropertyUtils#isSensitivePropertyKey} (the following 
token after {@code
+ *       --name} is always replaced with {@link 
HiddenPropertyMaskUtils#MASKED_VALUE}, including
+ *       non-secret values such as identifiers after {@code 
--use-credential-file})
+ *   <li>Standalone {@code -Dkey=value} when {@code key} is sensitive
+ * </ul>
+ *
+ * <p>Map fields {@code environments} / {@code customFields} / {@code configs} 
and runJob {@code
+ * jobConf} mask sensitive-named keys, plus nested JSON under {@link 
#NESTED_JSON_MAP_KEYS}.
+ *
+ * <p>Not masked (out of scope): resource URI fields ({@code executable}, 
{@code className}, {@code
+ * jars}, {@code files}, {@code archives}, Shell {@code scripts}); {@code 
--hiveconf}; conf property
+ * values that embed {@code -D} secrets under a non-sensitive conf key (for 
example {@code
+ * spark.driver.extraJavaOptions}); free-form tokens such as a {@code --where} 
SQL predicate.
+ *
+ * <p>Null map entries are preserved in the masked DTO / {@code jobConf} maps 
used on the REST and
+ * event {@code jobConf} paths. When {@code JobInfo} rebuilds a {@code 
JobTemplate} via {@code
+ * ImmutableMap.copyOf}, null values cannot be carried; a hand-edited runtime 
JSON with nulls may
+ * cause that rebuild to be skipped (logged) rather than expose a partial 
template.
+ */
+public final class JobRuntimeTemplateMaskUtils {
+
+  /**
+   * JobConf / template keys whose values are known to be JSON maps (Iceberg 
built-in jobs and the
+   * optimizer). Nested sensitive keys inside these values are masked.
+   *
+   * <ul>
+   *   <li>{@code updater_options} — update-stats job credentials / client 
options
+   *   <li>{@code spark_conf} — Spark configs for built-in Iceberg jobs
+   *   <li>{@code options} — rewrite-data-files / compaction options (from 
policy {@code jobOptions}
+   *       / {@code rewriteOptions})
+   * </ul>
+   */
+  static final Set<String> NESTED_JSON_MAP_KEYS =
+      ImmutableSet.of("updater_options", "spark_conf", "options");
+
+  /**
+   * Iceberg built-in job CLI flags whose following argument is a JSON map 
with the same masking
+   * rules as {@link #NESTED_JSON_MAP_KEYS}.
+   */
+  static final Set<String> NESTED_JSON_ARGUMENT_FLAGS =
+      ImmutableSet.of("--updater-options", "--spark-conf", "--options");
+
+  /**
+   * Spark flags whose payload is a single {@code key=value} config entry. 
Both the two-token form
+   * ({@code --conf key=value}) and the equals form ({@code --conf=key=value}) 
use {@link
+   * #maskConfKeyValuePayload}.
+   */
+  static final Set<String> CONF_ARGUMENT_FLAGS = ImmutableSet.of("--conf", 
"-c");

Review Comment:
   Fixed in 3b0447130.
   
   Skipping every payload that contains whitespace would leave a real secret in 
clear text, for example `-c "export db_password=pd && run.sh"`. `-c` stays in 
the conf flag set because Spark uses the same flag for `key=value`.
   
   A shell command is no longer cut at the first `=`. It is split on 
whitespace, each sensitive `key=value` token is masked on its own, and the rest 
of the command is kept:
   
   - `-c "export ACCOUNT_ID=5 && run.sh --flag"` → `-c "export 
ACCOUNT_ID=****** && run.sh --flag"` (`ACCOUNT_ID` still matches the existing 
`account` keyword, so the value is masked; `&& run.sh --flag` is kept)
   - `-c "DB_PASSWORD=pw run.sh"` → `-c "DB_PASSWORD=****** run.sh"`
   - `-c "echo ready && run.sh --flag"` is unchanged
   - Spark `-c jdbc.password=secret-pw` (no space) is still fully masked



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to