divyanshus2404 commented on code in PR #71711:
URL: https://github.com/apache/airflow/pull/71711#discussion_r4097419297
##########
providers/apache/spark/src/airflow/providers/apache/spark/hooks/spark_submit.py:
##########
@@ -516,26 +567,96 @@ def _get_spark_binary_path(self) -> list[str]:
def _mask_cmd(self, connection_cmd: str | list[str]) -> str:
# Mask any password related fields in application args with key value
pair
# where key contains password (case insensitive), e.g.
HivePassword='abc'
- connection_cmd_masked = re.sub(
- r"("
- r"\S*?" # Match all non-whitespace characters before...
- r"(?:secret|password)" # ...literally a "secret" or "password"
- # word (not capturing them).
- r"\S*?" # All non-whitespace characters before either...
- r"(?:=|\s+)" # ...an equal sign or whitespace characters
- # (not capturing them).
- r"(['\"]?)" # An optional single or double quote.
- r")" # This is the end of the first capturing group.
- r"(?:(?!\2\s).)*" # All characters between optional quotes
- # (matched above); if the value is quoted,
- # it may contain whitespace.
- r"(\2)", # Optional matching quote.
- r"\1******\3",
- " ".join(connection_cmd),
- flags=re.I,
- )
-
- return connection_cmd_masked
+ #
+ # Tokenise on whitespace so each token is examined once. For key=value
+ # tokens the unanchored _SENSITIVE_KV_RE handles multiple sensitive
keys
+ # inside a single token (e.g. ``Config(secret="x",password=y)``).
+ # Space-separated key/value pairs and quoted values that span multiple
+ # tokens are handled by manual lookahead. The overall scan is O(n).
+ if isinstance(connection_cmd, str):
+ connection_cmd = [connection_cmd]
+ cmd = " ".join(connection_cmd)
+ # re.split with a capturing group keeps the delimiters in the result
list,
+ # so we can reassemble the string without altering whitespace.
+ parts = re.split(r"(\s+)", cmd)
+ i = 0
+ while i < len(parts):
+ part = parts[i]
+ if part and not part.isspace() and
_SENSITIVE_KEYWORD_RE.search(part):
+ if "=" in part:
+ # key=value in the same token.
+ open_info = _find_open_quote_key(part)
+ if open_info is not None:
+ # The value's opening quote is not closed within this
token,
+ # so the quoted value spans multiple
whitespace-delimited
+ # tokens. Consume them until the closing quote, but
stop
+ # at newlines so an unterminated quote doesn't swallow
+ # subsequent log lines.
+ key_start, eq_idx, quote = open_info
+ before_key = part[:key_start]
+ key_eq = part[key_start : eq_idx + 1]
+
+ # Mask any earlier key=value pairs in the prefix.
+ if before_key and
_SENSITIVE_KEYWORD_RE.search(before_key):
+ before_key =
_SENSITIVE_KV_RE.sub(_mask_sensitive_kv, before_key)
+
+ found_close = False
+ j = i + 1
+ while j < len(parts):
+ if parts[j].isspace() and "\n" in parts[j]:
+ break
+ if not parts[j].isspace() and
parts[j].endswith(quote):
Review Comment:
Fixed in f66e620b: the token heuristics (including `endswith(quote)`) are
replaced by a scanner that closes quoted values exactly as `main` does; both
payloads are pinned in `test_masks_passwords`.
---
Drafted-by: Claude Code (Opus 5.5) (no human review before posting)
_🤖 Addressed by [Claude Code](https://claude.com/claude-code)_
##########
providers/apache/spark/src/airflow/providers/apache/spark/hooks/spark_submit.py:
##########
@@ -55,6 +55,57 @@
_K8S_WAIT_APP_COMPLETION_CONF = "spark.kubernetes.submission.waitAppCompletion"
+# Quick check for whether a token might contain a sensitive key.
+_SENSITIVE_KEYWORD_RE = re.compile(r"secret|password", re.IGNORECASE)
+
+# Per-token pattern for key=value forms where the value is fully contained in
+# the token. Intentionally *unanchored* so that it finds multiple sensitive
+# keys inside a single whitespace-delimited token (e.g.
+# ``Config(secret="x",password=y)``). Running it only on tokens that pass
+# the cheap ``_SENSITIVE_KEYWORD_RE`` check keeps the overall scan O(n).
+_SENSITIVE_KV_RE = re.compile(
+ r"(\S*?(?:secret|password)\S*?=)"
Review Comment:
Fixed in f66e620b: `_SENSITIVE_KV_RE` is gone and masking is a single linear
pass (`"a=" + "secret" * n` is 1.3 ms at n=80k), the O(n) comments now hold,
and a timing test on this payload is added.
---
Drafted-by: Claude Code (Opus 5.5) (no human review before posting)
_🤖 Addressed by [Claude Code](https://claude.com/claude-code)_
--
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]
