arxhr007 opened a new pull request, #76894:
URL: https://github.com/apache/airflow/pull/76894
`SecretsMasker.redact` only walks `dict`, `list`, `tuple` and `set`. Any
other container is returned unchanged, so a registered secret inside it reaches
the logs and rendered output in the clear. The same gap exists on the
fail-closed path for a sensitive key (`_redact_all`), which replaces the
contents of the four known containers and nothing else.
Reproduction on `main`, with `hunter2-secret` registered through `add_mask`:
| value | `redact(value)` on `main` |
|---|---|
| `frozenset({"hunter2-secret"})` | `frozenset({'hunter2-secret'})` |
| `collections.deque(["hunter2-secret"])` | `deque(['hunter2-secret'])` |
| `MappingProxyType({"k": "hunter2-secret"})` | `mappingproxy({'k':
'hunter2-secret'})` |
| `{"a": {"b": frozenset({"hunter2-secret"})}}` | secret unchanged |
This change walks `frozenset`, `collections.deque` and any
`collections.abc.Mapping` the same way as their `dict`, `list` and `tuple`
counterparts, on both `_redact` and `_redact_all`. `frozenset` is returned as a
tuple like `set`, `deque` as a list, and a non-`dict` mapping as a `dict`.
`dict` and `str` keep their existing early checks and the `Mapping` ABC is only
consulted last, so the common leaf types do not pay for it.
Not changed on purpose: dict keys are still not masked (a registered secret
equal to a key name such as `secret` would rename the key and change the output
structure, which `test_redact` rules out), and `bytes`, dataclasses and
arbitrary objects are still returned as they are.
##### Tests run
- `shared/secrets_masker/tests`: 212 passed, 1 skipped, 1 xfailed, 1 failed.
The failure is `test_redact_filehandles`, which fails identically on unmodified
`main` in my environment (Windows), so it is not caused by this change.
- The 5 new tests (`test_redact_masks_secret_in_container_types[...]` and
`test_redact_masks_every_leaf_of_other_container_types_under_a_sensitive_name`)
fail on `main` and pass with the change.
- `ruff check` and `ruff format --check` on `shared/secrets_masker`: passed.
- Not run: `prek`, mypy, `breeze verify`.
##### 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, `str` of 190 chars containing one secret, and a
structlog-style event dict with nested values; control is `sum(range(200))`):
| | main | this PR |
|---|---|---|
| `redact(str)` | 95.8-100.4 us | 95.2-100.2 us |
| `redact(event dict)` | 293-325 us | 298-313 us |
| control | 1.24-1.38 us | 1.28-1.37 us |
No change outside the noise. My first version checked `Mapping` before
`dict` and measured 6-9% slower on the event dict, which is why the ABC check
now comes last.
---
##### Was generative AI tooling used to co-author this PR?
- [X] Yes (please specify the tool below)
Generated-by: Claude Code (Sonnet 5.5) following [the
guidelines](https://github.com/apache/airflow/blob/main/contributing-docs/05_pull_requests.rst#gen-ai-assisted-contributions)
--
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]
