arxhr007 opened a new pull request, #76142:
URL: https://github.com/apache/airflow/pull/76142
`SecretsMasker` joins every registered secret into a single regex
alternation. Python's alternation takes the first alternative that matches, not
the longest, and `patterns` is a `set`, so its order depends on the process
hash seed. When one registered secret is a prefix of another, the longer secret
is sometimes only partially masked and its tail is written to the logs or
rendered output.
Reproduction on `main` (two secrets, `password1` is a prefix of
`password1234567890`):
```python
m = SecretsMasker()
m.add_mask("password1")
m.add_mask("password1234567890")
m.redact("token=password1234567890;")
# PYTHONHASHSEED=0 -> 'token=***;'
# PYTHONHASHSEED=3 -> 'token=***234567890;' <- tail of the secret leaks
```
This can happen with any pair of secrets sharing a prefix, for example a
Variable and a Connection password where one extends the other, or a value and
a `secret_mask_adapter` adaptation of it.
The fix compiles the alternation longest-first, so the longest registered
secret always wins. If `A` is a prefix of `B` then `re.escape(A)` is a prefix
of `re.escape(B)`, so sorting the escaped patterns by length is enough.
The regression test registers every prefix of one secret, which makes the
result independent of set iteration order (it fails on `main` for every hash
seed I tried, 1-3).
##### Tests run
- `shared/secrets_masker/tests`: 173 passed, 1 skipped, 1 xfailed under
`PYTHONHASHSEED` 0-4 and on the rebased branch
- `task-sdk/tests -k "mask or redact"`: 46 passed. One test,
`test_rendered_templates_masks_secrets_in_complex_objects`, fails identically
on `main` because the `kubernetes` package is not installed in my environment
- `prek run --stage pre-commit` on the changed files, including
`mypy-shared-secrets_masker`: passed
##### Performance
Secret masking is on the hot path of every log line, so I measured both
paths before and after (4 alternating runs each, best of 7 repeats, 200 random
secrets of 8-32 chars, 200-char line containing one secret; control is
`sum(range(200))`):
| | main | this PR |
|---|---|---|
| `redact(line)` (per log line) | 93.5-94.7 us | 90.5-92.7 us |
| registering 200 secrets with `add_mask` | 1.37-1.59 ms | 2.69-2.77 ms |
| control | 1.52-1.57 us | 1.51-1.59 us |
`redact` is unchanged within noise. `add_mask` is slower, about 7 us per
registered secret at 200 secrets, because it now sorts the patterns each time
it recompiles the regex. That runs once per newly masked secret, not per log
line, and recompiling the whole regex per secret was already the dominant cost.
---
##### Was generative AI tooling used to co-author this PR?
- [X] Yes — Devin CLI (Claude Sonnet 5.5)
Generated-by: Devin CLI (Claude Sonnet 5.5) following [the
guidelines](https://github.com/apache/airflow/blob/main/contributing-docs/05_pull_requests.rst#gen-ai-assisted-contributions)
---
* Read the **[Pull Request
Guidelines](https://github.com/apache/airflow/blob/main/contributing-docs/05_pull_requests.rst#pull-request-guidelines)**
for more information. Note: commit author/co-author name and email in commits
become permanently public when merged.
--
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]
