apache / apache/airflow

Deprecation warning for airflow.sdk.execution_time.secrets_masker points at the wrong module for mask_secret

Open Beginner friendly
#70,875 3 comments 0 reactions 0 assignees View on GitHub
area:core kind:bug
Dominant language
Python
Stars
46.9k
Forks
17.8k
Avg merge
2d 9h
Merged PRs (30d)
472

Description

### Apache Airflow version

3.2.2 (apache-airflow-task-sdk 1.2.2). Also present on `main` at 79fa2e622218138f1638fe3a225156e254b3e748.

### If "Other Airflow 2/3 version" selected, which one?

_No response_

### What happened?

The deprecation shim at `airflow.sdk.execution_time.secrets_masker` tells you to migrate to `airflow.sdk._shared.secrets_masker`. For `mask_secret` its own code deliberately does something else, and the two are not equivalent: `airflow.sdk.log.mask_secret` also forwards the secret to the supervisor, `airflow.sdk._shared.secrets_masker.mask_secret` does not. Following the warning silently drops supervisor masking.

The warning says `_shared` ([L33-L37](https://github.com/apache/airflow/blob/79fa2e622218138f1638fe3a225156e254b3e748/task-sdk/src/airflow/sdk/execution_time/secrets_masker.py#L33-L37)):

```python
warnings.warn(
"Importing from 'airflow.sdk.execution_time.secrets_masker' is deprecated and will be "
"removed in a future version. Please use 'airflow.sdk._shared.secrets_masker' instead.",
DeprecatedImportWarning,
stacklevel=2,
)
```

The code hands you `airflow.sdk.log` ([L43-L47](https://github.com/apache/airflow/blob/79fa2e622218138f1638fe3a225156e254b3e748/task-sdk/src/airflow/sdk/execution_time/secrets_masker.py#L43-L47)):

```python
def __getattr__(name: str):
if name == "mask_secret":
from airflow.sdk.log import mask_secret

return mask_secret
```

Confirmed by object identity on 3.2.2:

```
shim.mask_secret is airflow.sdk.log.mask_secret -> True
shim.mask_secret is airflow.sdk._shared.secrets_masker.mask_secret -> False
```

That special case is correct, and it is load bearing. `airflow.sdk.log.mask_secret` registers with the shared masker **and** notifies the supervisor ([log.py L250-L266](https://github.com/apache/airflow/blob/79fa2e622218138f1638fe3a225156e254b3e748/task-sdk/src/airflow/sdk/log.py#L250-L266)):

```python
def mask_secret(secret: JsonValue, name: str | None = None) -> None:
"""Mask a secret in both task process and supervisor process. ..."""
_secrets_masker().add_mask(secret, name)

with suppress(Exception):
from airflow.sdk.execution_time import task_runner
from airflow.sdk.execution_time.comms import MaskSecret

if comms := getattr(task_runner, "SUPERVISOR_COMMS", None):
comms.send(MaskSecret(value=secret, name=name))
```

The `_shared` one the warning recommends stops at the first line ([secrets_masker.py L92-…](https://github.com/apache/airflow/blob/79fa2e622218138f1638fe3a225156e254b3e748/shared/secrets_masker/src/airflow_shared/secrets_masker/secrets_masker.py#L92)); there is no `SUPERVISOR_COMMS` reference anywhere in that module. Under AIP-72 the supervisor is a separate process, so a secret registered only through `_shared` can still appear in supervisor log output.

The failure is silent. Both modules import, both expose `mask_secret`, both accept the call and return `None`. A test asserting the symbol is importable and callable passes against either one, including the one that misses the supervisor.

### What you think should happen instead?

The warning text and the module docstring should name `airflow.sdk.log` for `mask_secret`, matching what `__getattr__` already does. Either carve out the exception in the message, or drop the special case so the message becomes true.

The shim came in with https://github.com/apache/airflow/pull/54915, added defensively after https://github.com/apache/airflow/pull/54449 removed the path, so this reads as incidental rather than intended.

### How to reproduce

```python
import warnings
warnings.simplefilter("ignore")

import airflow.sdk.execution_time.secrets_masker as shim
import airflow.sdk._shared.secrets_masker as shared
import airflow.sdk.log as sdklog

print(shim.mask_secret is sdklog.mask_secret) # True, what the shim gives you
print(shim.mask_secret is shared.mask_secret) # False, what the warning tells you to use
```

### Operating System

macOS 15 (not OS specific)

### Versions of Apache Airflow Providers

_No response_

### Deployment

Other

### Deployment details

_No response_

### Anything else?

Two engineers migrating the same codebase independently read the deprecation message, moved to `airflow.sdk._shared.secrets_masker`, and ended up with weaker masking than they started with. Neither noticed, because every wrong answer in this area imports and runs fine. The human readable guidance is the only signal most people act on, and right now it points away from the implementation the shim itself chose.

### Are you willing to submit PR?

- [X] Yes I am willing to submit a PR!

### Code of Conduct

- [X] I agree to follow this project's [Code of Conduct](https://github.com/apache/airflow/blob/main/CODE_OF_CONDUCT.md)

Contributor guide

Open the contributing guide

Research direction

Start with task-sdk/src/airflow/sdk/execution_time/secrets_masker.py, especially the deprecation warning and __getattr__, then compare airflow.sdk.log.mask_secret with shared/secrets_masker/src/airflow_shared/secrets_masker/secrets_masker.py. Run the reproduction snippet from the issue to confirm the symbol identity. Done means the warning and module docstring direct mask_secret users to airflow.sdk.log without removing the existing special case.

Written by the indexing model from the issue text.

Assessment

Tech stack
python
Domain
backend, security
Issue type
Bug
Difficulty
2/5
Estimated time
1-3 hours
Activity status
Quiet
Clarity
Clearly specified
Newbie friendliness
75/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.