[Newer1107]

Problem

The _mask_cmd() method in SparkSubmitHook uses a regex with a nested lookahead (?:(?!\2\s).)* combined with \S*? that causes quadratic backtracking (O(n²)) on inputs without the secret/password pattern. An authenticated user can trigger a DoS by passing crafted application_args via the DAG trigger API.

Input size	Before	After
10,000 chars	~2s	<1ms
50,000 chars	~57s	<1ms

Fix

Replace the vulnerable regex with:

• \b\w* for the key part (word boundary limits search space)
• \S+ for the value (no backtracking possible)

This eliminates catastrophic backtracking while preserving the same masking behavior.

Verification

Tested with:

• HivePassword=abc123 → HivePassword=******
• secret=mysecret → secret=******
• password="mypassword" → password="******"
• 50,000 char input: <1ms (was 57s)

Resolves #70676

[boring-cyborg]

Congratulations on your first Pull Request and welcome to the Apache Airflow community! If you have any issues or are unsure about any anything please check our Contributors' Guide
Here are some useful points:

• Pay attention to the quality of your code (ruff, mypy and type annotations). Our prek-hooks will help you with that.
• In case of a new feature add useful documentation (in docstrings or in docs/ directory). Adding a new operator? Check this short guide Consider adding an example Dag that shows how users should use it.
• Consider using Breeze environment for testing locally, it's a heavy docker but it ships with a working Airflow and a lot of integrations.
• Be patient and persistent. It might take some time to get a review or get the final approval from Committers.
• Please follow ASF Code of Conduct for all communication including (but not limited to) comments on Pull Requests, Mailing list and Slack.
• Be sure to read the Airflow Coding style.
• Always keep your Pull Requests rebased, otherwise your build might fail due to changes not related to your commits.
  Apache Airflow is a community-driven project and together we are making it better 🚀.
  In case of doubts contact the developers at:
  Mailing List: dev@airflow.apache.org
  Slack: https://s.apache.org/airflow-slack

[potiuk]

Thanks for tracking this down — the quadratic behaviour is real. I measured the current regex at 0.03s / 0.19s / 0.70s for 2k / 5k / 10k-character tokens, so the growth you describe is exactly right. But the rewrite changes what gets masked, and one of those changes writes credentials to the task log in clear text, so this needs another pass before it can go in.

Blocking: dotted and hyphenated secret keys are no longer masked

Description doesn't match code: PR description describes something different from what the code actually does.

connection_cmd_masked = re.sub(
    r"(\b\w*(?:secret|password)\w*(?:=|\s+))"  # key part (word boundary prevents catastrophic backtracking)
    r"(['\"]?)"  # optional opening quote
    r"(\S+)"  # value (non-space, no backtracking)

\w* cannot cross . or -, so a key where secret or password is a dotted or hyphenated segment no longer matches and the value is logged in the clear. I ran the old and new patterns side by side:

Input	current main	this PR
--conf spark.hadoop.fs.s3a.secret.key=AKIAxyz	...secret.key=******	...secret.key=AKIAxyz
--secret-key=abc	--secret-key=******	--secret-key=abc

The first one is the standard S3A secret-key conf — probably the most common secret that goes through spark-submit --conf. The original \S*? was deliberately broad to cover these.

You can keep the original semantics and kill the quadratic scan by anchoring the match at a token boundary instead of rewriting the key grammar. (?<!\S) makes each token get scanned exactly once, and because of leftmost-match the old regex already effectively started at a token boundary, so behaviour is unchanged:

r"((?<!\S)\S*?(?:secret|password)\S*?(?:=|\s+)(['\"]?))(?:(?!\2\s).)*(\2)"

with the original replacement r"\1******\3". I checked this against the 8 existing test_masks_passwords cases plus the ones below: zero differences from the current behaviour, and a 10k-character single token goes from 0.70s to 0.0003s.

Major: quoted values containing whitespace now partially leak

    r"(['\"]?)"  # optional opening quote
    r"(\S+)"  # value (non-space, no backtracking)
    r"(\2)",  # optional closing quote
    r"\1\2******\2",

The comment on the old value part said "if the value is quoted, it may contain whitespace"; \S+ drops that:

Input	current main	this PR
--password='my secret pass' --foo bar	--password='******' --foo bar	--password=****** secret ****** --foo bar
password="my pass" next	password="******" next	password=****** pass" next
--password=	--password=******	not matched

The engine falls back to an empty quote group and treats 'my as the value, so the middle of the secret survives. The anchored pattern above keeps all of these.

Major: no regression test

For bug-fix PRs, flag if there is no regression test — a test that fails without the fix and passes with it.

test_masks_passwords is untouched, and all 8 of its cases pass with the new regex — so CI would have been green while the two regressions above shipped. Please add:

1. a regression test for the slowdown itself, e.g. mask "--conf " + "x" * 40_000 — with the current regex that takes seconds, with a fixed one under a millisecond, so a modest wall-clock bound (or simply the test completing) fails without the fix;
2. cases pinning the behaviours this PR broke, so they cannot regress again: --conf spark.hadoop.fs.s3a.secret.key=AKIAxyz → ...secret.key=******, --secret-key=abc → --secret-key=******, --password='my secret pass' --foo bar → --password='******' --foo bar.

Smaller observations

• The per-line comments ("optional opening quote", "optional closing quote", "value (non-space...)") restate the regex. The one thing worth recording is why the pattern must not go back to an unanchored \S*? prefix — a single comment above re.sub carries that; the rest can go.
• The PR body does not use the repository template, so the "Was generative AI tooling used to co-author this PR?" section is missing rather than unchecked. Please restore it and answer it either way; see the Gen-AI section of contributing-docs/05_pull_requests.rst.
• On the framing: the slow path is only reachable via application_args if a Dag author pipes run parameters straight into it, and Dag authors are trusted in Airflow's security model — so this is a worthwhile robustness fix rather than a vulnerability in the hook. No urgency, just setting expectations.
• Note the main CI workflow has not run on this PR yet, so nothing here is verified by CI; a maintainer will approve the workflow run once the above is addressed.

---

This review was drafted by an AI-assisted tool and
confirmed by an Apache Airflow maintainer. After you've
addressed the points above and pushed an update, an Apache Airflow
maintainer — a real person — will take the next look
at the PR. The findings cite the project's review criteria;
if you think one of them is mis-applied, please reply on the
PR and a maintainer will weigh in.

More on how Apache Airflow handles maintainer review:
contributing-docs/05_pull_requests.rst.

[potiuk]

Closing this in favour of #71711, which fixes the same quadratic-backtracking problem in _mask_cmd. Thanks for spotting the issue — the diagnosis was right, and the slowdown is real (I measured the current pattern at 0.03s / 0.19s / 0.70s for 2k / 5k / 10k-character tokens).

I ran all three patterns — current main, this PR, and #71711 — over the same inputs:

input	main	this PR	#71711
--conf spark.hadoop.fs.s3a.secret.key=AKIAxyz	masked	leaks	same as main
--secret-key=abc	masked	leaks	same as main
--password='my secret pass' --foo bar	--password='******' --foo bar	partial leak	same as main
password="my pass" next	password="******" next	partial leak	same as main
--password=	masked	unmasked	same as main

The root cause is in the review above: \b\w* cannot cross . or -, so dotted and hyphenated keys stop matching, and (\S+) drops the old "quoted values may contain whitespace" handling. #71711 preserves main's output byte-for-byte on all of these while getting the same speedup, and it arrived first with tests and a round of review.

If you would like to keep working in this area, the anchored one-liner from my review above is still a valid alternative approach, and #71711 has one open gap of its own (a second sensitive key inside the same whitespace-free token is not masked) that I have asked its author to close — a review there would be welcome.

---

This message was drafted by an AI-assisted tool and
confirmed by an Apache Airflow maintainer. If you think
something here is mistaken, please reply on the PR and a
maintainer will weigh in.

More on how Apache Airflow handles maintainer review:
contributing-docs/05_pull_requests.rst.