RehanAhmad25 commented on PR #72499: URL: https://github.com/apache/airflow/pull/72499#issuecomment-5550022031
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. -- 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]
