Skip to content

Mask password-format Param values in conf, logs, and rendered templates - #72499

Open
RehanAhmad25 wants to merge 2 commits into
apache:mainfrom
RehanAhmad25:fix/72377-mask-password-params
Open

Mask password-format Param values in conf, logs, and rendered templates#72499
RehanAhmad25 wants to merge 2 commits into
apache:mainfrom
RehanAhmad25:fix/72377-mask-password-params

Conversation

@RehanAhmad25

Copy link
Copy Markdown

Adds format="password" as a supported JSON Schema keyword for Param, and closes the loop on where that value ends up plaintext today.

The trigger-form input side of this already worked (FieldPassword.tsx already masks the input whenever schema.format == "password"), but nothing downstream of that respected the declaration. This PR:

  • Registers password-format param values with mask_secret() in process_params() (task-sdk), so they're redacted in Task Logs and Rendered Templates. Verified this in source: _serialize_rendered_fields() in task-sdk/src/airflow/sdk/execution_time/task_runner.py calls redact() against the same masker singleton, in the worker, before rendered fields are sent to the API server.
  • Redacts conf in the DAG Run Details page and REST API: a helper checks each top-level conf key against its DAG's declared Param schema and replaces the value with *** when format == "password". Applied to get_dag_run, both pagination branches of get_dag_runs (including the dag_id="~" all-DAGs case, resolved per-run against that run's own DAG), and get_list_dag_runs_batch.

Deliberately out of scope for this PR (discussed on the issue):

  • XCom display a value pushed to XCom isn't touched by any of the above, since neither the XCom model nor its API routes run values through redact(). This is the same known limitation that already applies to Connection.password today (confirmed: _mask_connection_secrets() masks it "from logs" per its own docstring, nothing else).
  • Fernet encryption of conf at rest in the metadata DB is flagged as a separate, larger discussion rather than folded into this fix.
  • Nested object schemas with their own format on a sub-property format="password" is only honored on string-typed params for now.

Docs for format="password" in core-concepts/params.rst will follow in a follow-up commit on this PR (or a fast-follow), explicitly stating what's covered and what isn't so nobody assumes broader coverage than what's implemented.

Closes: #72377


Was generative AI tooling used to co-author this PR?
  • Yes (please specify the tool below)

Generated-by: Claude (Anthropic) following the guidelines

@fat-catTW

Copy link
Copy Markdown
Contributor

Thanks for contribution!

One thing I noticed in the task-log masking path: when [core] dag_run_conf_overrides_params is enabled, process_params() logs the raw dagrun_conf before password-format values are registered with mask_secret().

if conf.getboolean("core", "dag_run_conf_overrides_params") and dagrun_conf:
    logger.debug("Updating task params (%s) with DagRun.conf (%s)", params, dagrun_conf)
    params.update(dagrun_conf)

resolved_params = params.validate()
_mask_password_params(params, resolved_params)
return resolved_params

This means a password-format value from DagRun.conf can be sent to the logging path before it has been registered with the secrets masker. For example, a run triggered with {"api_token": "super-secret-value"} for a Param(..., type="string", format="password") could still emit a debug log containing the raw "super-secret-value".

Could we either register password-format values before this log line, or avoid logging raw dagrun_conf here?

@RehanAhmad25
RehanAhmad25 force-pushed the fix/72377-mask-password-params branch from 8b59be9 to 13ee14c Compare September 5, 2026 06:33
@RehanAhmad25

Copy link
Copy Markdown
Author

Thanks for the review @fat-catTW .
It is a good catch, that's a real bug. Confirmed it in source: ParamsDict.__setitem__ preserves the same Param object (and its declared schema) when you set a value for an already-declared key, it only creates a bare Param for undeclared keys. That means dag.params/task.params are already merged into params by the time we reach the dag_run_conf_overrides_params branch, so the declared format="password" schema for a key is available even before dagrun_conf itself gets merged in.

Fixed by masking against the raw dagrun_conf dict right before the debug log line, using that already-present schema, instead of only masking the final resolved params after params.validate(). Renamed the helper to _mask_password_values() since it now gets called twice: once on dagrun_conf before the log line, and once on the final resolved_params afterward (covers password-format defaults that aren't overridden by conf at all).

Added a regression test (test_masks_dagrun_conf_value_before_debug_log) that asserts mask_secret() gets called before logger.debug() in this path, so this can't silently regress later. Pushed as a follow-up commit.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:API Airflow's REST/HTTP API area:task-sdk

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Support masking secrets in DagRun params

2 participants